[lttng-dev] [PATCH] Enforce exactly one LTTNG_DOMAIN (add_context.c) (updated) (REPLACES PREVIOUS PATCH)

[Thibault, Daniel]

>From 43aaf8dc6e9f38d31e66afb8e8f0c218fb32a07e Thu, 9 Feb 2012 17:36:21 -0500
From: Daniel U. Thibault <daniel.thibault at drdc-rddc.gc.ca>
Date: Thu, 9 Feb 2012 17:36:01 -0500
Subject: [PATCH] Enforce exactly one LTTNG_DOMAIN (add_context.c) (updated)

Also fix a memory leak.

Signed-off-by: Daniel U. Thibault <daniel.thibault at drdc-rddc.gc.ca>

diff --git a/src/bin/lttng/commands/add_context.c b/src/bin/lttng/commands/add_context.c
index e499ef3..dc8de4b 100644
--- a/src/bin/lttng/commands/add_context.c
+++ b/src/bin/lttng/commands/add_context.c
@@ -272,7 +272,7 @@
 };
 
 /*
- * Pretty print context type.
+ * Pretty print context type (FILE *ofp must be valid).
  */
 static void print_ctx_type(FILE *ofp)
 {
@@ -309,6 +309,7 @@
 	fprintf(ofp, "will be added to all events and all channels.\n");
 	fprintf(ofp, "Otherwise the context will be added only to the channel (-c)\n");
 	fprintf(ofp, "and/or event (-e) indicated.\n");
+	fprintf(ofp, "Exactly one domain (-k/--kernel or -u/--userspace) must be specified.\n");
 	fprintf(ofp, "\n");
 	fprintf(ofp, "Options:\n");
 	fprintf(ofp, "  -h, --help               Show this help\n");
@@ -325,9 +326,8 @@
 #else
 	fprintf(ofp, "  -u, --userspace          Apply to the user-space tracer\n");
 #endif
-	fprintf(ofp, "  -t, --type TYPE          Context type. You can repeat that option on\n");
-	fprintf(ofp, "                           the command line to specify multiple contexts at once.\n");
-	fprintf(ofp, "                           (--kernel preempts --userspace)\n");
+	fprintf(ofp, "  -t, --type TYPE          Context type. You can repeat that option on the\n");
+	fprintf(ofp, "                           command line to specify multiple contexts at once.\n");
 	fprintf(ofp, "                           TYPE can be one of the strings below:\n");
 	print_ctx_type(ofp);
 	fprintf(ofp, "\n");
@@ -340,7 +340,8 @@
 }
 
 /*
- * Find context numerical value from string.
+ * Find context numerical value from string (which must not be NULL).
+ * Returns the index on success, -1 on failure.
  */
 static int find_ctx_type_idx(const char *opt)
 {
@@ -360,6 +361,7 @@
 
 /*
  * Add context to channel or event.
+ * Returns a CMD_* result value.
  */
 static int add_context(char *session_name)
 {
@@ -372,19 +374,21 @@
 	memset(&context, 0, sizeof(context));
 	memset(&dom, 0, sizeof(dom));
 
+	/* cmd_add_context() ensures exactly one domain is set */
 	if (opt_kernel) {
 		dom.type = LTTNG_DOMAIN_KERNEL;
 	} else if (opt_userspace) {
 		dom.type = LTTNG_DOMAIN_UST;
 	} else {
-		ERR("Please specify a tracer (-k/--kernel or -u/--userspace)");
-		ret = CMD_ERROR;
+		/* TODO As LTTNG_DOMAIN_* values are added, capture them here */
+		/* This can be reached only if the TODO hasn't been done */
+		ret = CMD_FATAL;
 		goto error;
 	}
 
 	handle = lttng_create_handle(session_name, &dom);
 	if (handle == NULL) {
-		ret = CMD_ERROR;
+		ret = CMD_FATAL;
 		goto error;
 	}
 
@@ -431,23 +435,25 @@
 		}
 	}
 
-	ret = CMD_SUCCESS;
+	/*
+	 * CMD_WARNING means that at least one lttng_add_context() failed and
+	 * tells the user to look on stderr for error(s).
+	 */
+	if (warn) {
+		ret = CMD_WARNING;
+	} else {
+		ret = CMD_SUCCESS;
+	}
 
 error:
 	lttng_destroy_handle(handle);
 
-	/*
-	 * This means that at least one add_context failed and tells the user to
-	 * look on stderr for error(s).
-	 */
-	if (warn) {
-		ret = CMD_WARNING;
-	}
 	return ret;
 }
 
 /*
  * Add context to channel or event.
+ * Returns a CMD_* result value.
  */
 int cmd_add_context(int argc, const char **argv)
 {
@@ -522,18 +528,30 @@
 		goto end;
 	}
 
-	if (!opt_session_name) {
-		session_name = get_session_name();
-		if (session_name == NULL) {
-			ret = CMD_ERROR;
-			goto end;
-		}
-	} else {
-		session_name = opt_session_name;
+	/* Enforce requirement for exactly one domain */
+	/* Five LTTNG_DOMAIN_* values are currently planned */
+	if (opt_kernel && opt_userspace) {
+		ERR("More than one domain specified");
+		ret = CMD_ERROR;
+		goto end;
+	} else if (!opt_kernel && !opt_userspace) {
+		ERR("No domain specified: Use one of -k/--kernel or -u/--userspace");
+		ret = CMD_ERROR;
+		goto end;
+	}
+
+	session_name = (opt_session_name ? opt_session_name : get_session_name() );
+	if (session_name == NULL) {
+		ret = CMD_ERROR;
+		goto end;
 	}
 
 	ret = add_context(session_name);
 
+	if (!opt_session_name) {
+		free (session_name);
+	}
+
 end:
 	/* Cleanup allocated memory */
 	cds_list_for_each_entry_safe(type, tmptype, &ctx_type_list.head, list) {
------------------------------

Daniel U. Thibault
R & D pour la défense Canada - Valcartier (RDDC Valcartier) / Defence R&D Canada - Valcartier (DRDC Valcartier)
Système de systèmes (SdS) / System of Systems (SoS)
Solutions informatiques et expérimentations (SIE) / Computing Solutions and Experimentations (CSE)
2459 Boul. Pie XI Nord
Québec, QC  G3J 1X5
CANADA
Vox : (418) 844-4000 x4245
Fax : (418) 844-4538
NAC: 918V QSDJ
Gouvernement du Canada / Government of Canada
<http://www.valcartier.drdc-rddc.gc.ca/>