[lttng-dev] [PATCH] enable_channels.c : Document and enforce return values, enforce exactly one domain flag, adjust usage() output accordingly, handle channel list errors.

[Thibault, Daniel]

>From 6c3fd4d3fa58f59f6eaa89a9add279a5581e42c5 Thu, 2 Feb 2012 15:14:19 -0500
From: Daniel U. Thibault <daniel.thibault at drdc-rddc.gc.ca>
Date: Thu, 2 Feb 2012 15:14:07 -0500
Subject: [PATCH] enable_channels.c : Document and enforce return values, enforce exactly one domain flag, adjust usage() output accordingly, handle channel list errors.

Also some enable_events minor corrections.

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

diff --git a/src/bin/lttng/commands/enable_channels.c b/src/bin/lttng/commands/enable_channels.c
index 692480e..371a74d 100644
--- a/src/bin/lttng/commands/enable_channels.c
+++ b/src/bin/lttng/commands/enable_channels.c
@@ -81,6 +81,7 @@
 static void usage(FILE *ofp)
 {
 	fprintf(ofp, "usage: lttng enable-channel NAME[,NAME2,...] [options] [channel_options]\n");
+	fprintf(ofp, "Exactly one domain (-k/--kernel or -u/--userspace) must be specified.\n");
 	fprintf(ofp, "\n");
 	fprintf(ofp, "  -h, --help               Show this help\n");
 	fprintf(ofp, "      --list-options       Simple listing of options\n");
@@ -89,7 +90,7 @@
 #if 0
 	fprintf(ofp, "  -u, --userspace [CMD]    Apply to the user-space tracer\n");
 	fprintf(ofp, "                           If no CMD, the domain used is UST global\n");
-	fprintf(ofp, "                           or else the domain is UST EXEC_NAME\n");
+	fprintf(ofp, "                           otherwise the domain is UST EXEC_NAME\n");
 	fprintf(ofp, "  -p, --pid PID            If -u, apply to specific PID (domain: UST PID)\n");
 #else
 	fprintf(ofp, "  -u, --userspace          Apply to the user-space tracer\n");
@@ -148,6 +149,7 @@
 
 /*
  * Adding channel using the lttng API.
+ * Returns a CMD_* result value.
  */
 static int enable_channel(char *session_name)
 {
@@ -156,29 +158,29 @@
 	struct lttng_domain dom;
 
 	/* Create lttng domain */
+	/* cmd_enable_channels() enforces the existence of just one domain flag */
 	if (opt_kernel) {
 		dom.type = LTTNG_DOMAIN_KERNEL;
-	} else if (opt_userspace) {
+	} else { /* if (opt_userspace) { */
 		dom.type = LTTNG_DOMAIN_UST;
-	} else {
-		ERR("Please specify a tracer (-k/--kernel or -u/--userspace)");
-		ret = CMD_ERROR;
-		goto error;
+	/* TODO As LTTNG_DOMAIN_* values are added, capture them here */
 	}
 
 	set_default_attr(&dom);
 
 	handle = lttng_create_handle(session_name, &dom);
 	if (handle == NULL) {
-		ret = -1;
+		ret = CMD_FATAL;
 		goto error;
 	}
 
 	/* Strip channel list (format: chan1,chan2,...) */
 	channel_name = strtok(opt_channels, ",");
+	/* Default to a warning in case opt_channels is empty */
+	ret = CMD_WARNING;
 	while (channel_name != NULL) {
 		/* Copy channel name and normalize it */
-		strncpy(chan.name, channel_name, NAME_MAX);
+		strncpy(chan.name, channel_name, NAME_MAX-1);
 		chan.name[NAME_MAX - 1] = '\0';
 
 		DBG("Enabling channel %s", channel_name);
@@ -193,12 +195,15 @@
 					opt_kernel ? "Kernel" : "UST", channel_name,
 					session_name);
 		}
+		ret = CMD_SUCCESS;
 
 		/* Next event */
 		channel_name = strtok(NULL, ",");
 	}
-
-	ret = CMD_SUCCESS;
+	if (ret == CMD_WARNING) {
+		ERR("No channel specified for %s session %s",
+				(opt_kernel ? "kernel" : "UST"), session_name);
+	}
 
 error:
 	if (warn) {
@@ -224,6 +229,7 @@
 
 /*
  * Add channel to trace session
+ * Returns a CMD_* result value.
  */
 int cmd_enable_channels(int argc, const char **argv)
 {
@@ -282,6 +288,18 @@
 		}
 	}
 
+	/* Enforce requirement for exactly one domain */
+	/* Five LTTNG_DOMAIN_* values are currently planned */
+	if (!opt_userspace && !opt_kernel) {
+		ERR("No domain specified: Use one of -k/--kernel or -u/--userspace");
+		ret = CMD_ERROR;
+		goto end;
+	} else if (opt_userspace && opt_kernel) {
+		ERR("More than one domain specified");
+		ret = CMD_ERROR;
+		goto end;
+	}
+
 	opt_channels = (char*) poptGetArg(pc);
 	if (opt_channels == NULL) {
 		ERR("Missing channel name.\n");
@@ -290,14 +308,10 @@
 		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;
+	session_name = opt_session_name ? opt_session_name : get_session_name();
+	if (session_name == NULL) {
+		ret = CMD_ERROR;
+		goto end;
 	}
 
 	ret = enable_channel(session_name);
diff --git a/src/bin/lttng/commands/enable_events.c b/src/bin/lttng/commands/enable_events.c
index 24cd286..1fcf741 100644
--- a/src/bin/lttng/commands/enable_events.c
+++ b/src/bin/lttng/commands/enable_events.c
@@ -303,7 +303,7 @@
 	struct lttng_domain dom;
 
 	/* Create lttng domain */
-	/* cmd_add_context() enforces the existence of exactly one domain flag */
+	/* cmd_enable_events() enforces the existence of exactly one domain flag */
 	if (opt_kernel) {
 		dom.type = LTTNG_DOMAIN_KERNEL;
 	} else { /* if (opt_userspace) { */
@@ -321,7 +321,6 @@
 		channel_name = opt_channel_name;
 	}
 
-	handle = NULL;
 	handle = lttng_create_handle(session_name, &dom);
 	if (handle == NULL) {
 		ret = CMD_FATAL;
@@ -495,8 +494,8 @@
 		} else {
 			MSG("%s event %s created in channel %s",
 					opt_kernel ? "kernel": "UST", event_name, channel_name);
-			ret = CMD_SUCCESS;
 		}
+		ret = CMD_SUCCESS;
 
 		/* Next event */
 		event_name = strtok(NULL, ",");
------------------------------

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/>