[lttng-dev] [PATCH] Fix alignment and wording of usage() output, setup some TODOs (Replacing all the deferred patches)

Thibault, Daniel Daniel.Thibault at drdc-rddc.gc.ca
Wed Feb 1 17:50:55 EST 2012


>From 1b5ba907a4910bd10dda3fbd14c1f1751893e268 Wed, 1 Feb 2012 17:48:14 -0500
From: Daniel U. Thibault <daniel.thibault at drdc-rddc.gc.ca>
Date: Wed, 1 Feb 2012 17:47:18 -0500
Subject: [PATCH] Fix alignment and wording of usage() output, setup some TODOs

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

diff --git a/src/bin/lttng/commands/calibrate.c b/src/bin/lttng/commands/calibrate.c
index 547b349..2328e44 100644
--- a/src/bin/lttng/commands/calibrate.c
+++ b/src/bin/lttng/commands/calibrate.c
@@ -101,15 +101,13 @@
 	fprintf(ofp, "\n");
 	fprintf(ofp, "Calibrate options:\n");
 #if 0
-	fprintf(ofp, "    --tracepoint           Tracepoint event (default)\n");
-	fprintf(ofp, "    --probe\n");
-	fprintf(ofp, "                           Dynamic probe.\n");
+	fprintf(ofp, "    --tracepoint            Tracepoint event (default)\n");
+	fprintf(ofp, "    --probe                 Dynamic probe.\n");
 #if 0
-	fprintf(ofp, "    --function:entry symbol\n");
-	fprintf(ofp, "                           Function tracer event\n");
+	fprintf(ofp, "    --function:entry symbol Function tracer event\n");
 #endif
-	fprintf(ofp, "    --syscall              System call eventl\n");
-	fprintf(ofp, "    --marker               User-space marker (deprecated)\n");
+	fprintf(ofp, "    --syscall               System call eventl\n");
+	fprintf(ofp, "    --marker                User-space marker (deprecated)\n");
 #else
 	fprintf(ofp, "    --function             Dynamic function entry/return probe (default)\n");
 #endif
@@ -127,6 +125,7 @@
 	struct lttng_domain dom;
 	struct lttng_calibrate calibrate;
 
+	/* TODO Enforce requirement of exactly one domain in cmd_...() */
 	/* Create lttng domain */
 	if (opt_kernel) {
 		dom.type = LTTNG_DOMAIN_KERNEL;
@@ -138,6 +137,7 @@
 		goto error;
 	}
 
+	/* TODO Upgrade this failure to CMD_FATAL? */
 	handle = lttng_create_handle(NULL, &dom);
 	if (handle == NULL) {
 		ret = CMD_ERROR;
@@ -172,6 +172,7 @@
 		goto error;
 	}
 
+	/* TODO Remove this unnecessary assignment */
 	ret = CMD_SUCCESS;
 
 error:
diff --git a/src/bin/lttng/commands/destroy.c b/src/bin/lttng/commands/destroy.c
index b0262ea..6203c21 100644
--- a/src/bin/lttng/commands/destroy.c
+++ b/src/bin/lttng/commands/destroy.c
@@ -52,7 +52,7 @@
 	fprintf(ofp, "get it from the configuration directory (.lttng).\n");
 	fprintf(ofp, "\n");
 	fprintf(ofp, "  -h, --help           Show this help\n");
-	fprintf(ofp, "      --list-options       Simple listing of options\n");
+	fprintf(ofp, "      --list-options   Simple listing of options\n");
 	fprintf(ofp, "\n");
 }
 
diff --git a/src/bin/lttng/commands/disable_channels.c b/src/bin/lttng/commands/disable_channels.c
index 05ee0e6..423d867 100644
--- a/src/bin/lttng/commands/disable_channels.c
+++ b/src/bin/lttng/commands/disable_channels.c
@@ -70,15 +70,15 @@
 	fprintf(ofp, "\n");
 	fprintf(ofp, "  -h, --help               Show this help\n");
 	fprintf(ofp, "      --list-options       Simple listing of options\n");
-	fprintf(ofp, "  -s, --session            Apply on session name\n");
-	fprintf(ofp, "  -k, --kernel             Apply for the kernel tracer\n");
+	fprintf(ofp, "  -s, --session NAME       Apply to session name\n");
+	fprintf(ofp, "  -k, --kernel             Apply to the kernel tracer\n");
 #if 0
-	fprintf(ofp, "  -u, --userspace [CMD]    Apply for the user-space tracer\n");
+	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 for the user-space tracer\n");
+	fprintf(ofp, "  -u, --userspace          Apply to the user-space tracer\n");
 #endif
 	fprintf(ofp, "\n");
 }
@@ -92,6 +92,7 @@
 	char *channel_name;
 	struct lttng_domain dom;
 
+	/* TODO Enforce requirement of exactly one domain in cmd_...() */
 	/* Create lttng domain */
 	if (opt_kernel) {
 		dom.type = LTTNG_DOMAIN_KERNEL;
@@ -102,7 +103,7 @@
 		ret = CMD_ERROR;
 		goto error;
 	}
-
+	/* TODO Document and enforce return values (e.g. -1 should be CMD_ERROR) */
 	handle = lttng_create_handle(session_name, &dom);
 	if (handle == NULL) {
 		ret = -1;
@@ -111,9 +112,12 @@
 
 	/* Strip channel list */
 	channel_name = strtok(opt_channels, ",");
+	/* TODO Issue CMD_WARNING if some lttng_disable_channel() calls fail */
+	/* TODO or if opt_channels is effectively empty */
 	while (channel_name != NULL) {
 		DBG("Disabling channel %s", channel_name);
 
+		/* TODO setup a chan_name[LTTNG_SYMBOL_NAME_LEN] filter */
 		ret = lttng_disable_channel(handle, channel_name);
 		if (ret < 0) {
 			goto error;
@@ -186,6 +190,7 @@
 	ret = disable_channels(session_name);
 
 end:
+	/* TODO Fix memory leak of session_name when opt_session_name == NULL */
 	poptFreeContext(pc);
 	return ret;
 }
diff --git a/src/bin/lttng/commands/disable_events.c b/src/bin/lttng/commands/disable_events.c
index 4ddcf54..98901db 100644
--- a/src/bin/lttng/commands/disable_events.c
+++ b/src/bin/lttng/commands/disable_events.c
@@ -74,17 +74,17 @@
 	fprintf(ofp, "\n");
 	fprintf(ofp, "  -h, --help               Show this help\n");
 	fprintf(ofp, "      --list-options       Simple listing of options\n");
-	fprintf(ofp, "  -s, --session            Apply on session name\n");
-	fprintf(ofp, "  -c, --channel            Apply on this channel\n");
+	fprintf(ofp, "  -s, --session NAME       Apply to session name\n");
+	fprintf(ofp, "  -c, --channel NAME       Apply to this channel\n");
 	fprintf(ofp, "  -a, --all-events         Disable all tracepoints\n");
-	fprintf(ofp, "  -k, --kernel             Apply for the kernel tracer\n");
+	fprintf(ofp, "  -k, --kernel             Apply to the kernel tracer\n");
 #if 0
 	fprintf(ofp, "  -u, --userspace [CMD]    Apply for 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 for the user-space tracer\n");
+	fprintf(ofp, "  -u, --userspace          Apply to the user-space tracer\n");
 #endif
 	fprintf(ofp, "\n");
 }
@@ -110,6 +110,7 @@
 		channel_name = opt_channel_name;
 	}
 
+	/* TODO Enforce requirement of exactly one domain in cmd_...() */
 	if (opt_kernel && opt_userspace) {
 		ERR("Can't use -k/--kernel and -u/--userspace together");
 		ret = CMD_FATAL;
@@ -127,6 +128,7 @@
 		goto error;
 	}
 
+	/* TODO Document and enforce return values (e.g. -1 should be CMD_FATAL) */
 	handle = lttng_create_handle(session_name, &dom);
 	if (handle == NULL) {
 		ret = -1;
@@ -146,6 +148,8 @@
 
 	/* Strip event list */
 	event_name = strtok(opt_event_list, ",");
+	/* TODO Issue CMD_WARNING if some lttng_disable_event() calls fail */
+	/* TODO or if opt_event_list is effectively empty */
 	while (event_name != NULL) {
 		/* Kernel tracer action */
 		if (opt_kernel) {
@@ -244,6 +248,7 @@
 	ret = disable_events(session_name);
 
 end:
+	/* TODO Fix memory leak of session_name when opt_session_name == NULL */
 	poptFreeContext(pc);
 	return ret;
 }
diff --git a/src/bin/lttng/commands/enable_channels.c b/src/bin/lttng/commands/enable_channels.c
index 19b6b26..51f4248 100644
--- a/src/bin/lttng/commands/enable_channels.c
+++ b/src/bin/lttng/commands/enable_channels.c
@@ -84,15 +84,15 @@
 	fprintf(ofp, "\n");
 	fprintf(ofp, "  -h, --help               Show this help\n");
 	fprintf(ofp, "      --list-options       Simple listing of options\n");
-	fprintf(ofp, "  -s, --session            Apply on session name\n");
-	fprintf(ofp, "  -k, --kernel             Apply on the kernel tracer\n");
+	fprintf(ofp, "  -s, --session NAME       Apply to session name\n");
+	fprintf(ofp, "  -k, --kernel             Apply to the kernel tracer\n");
 #if 0
-	fprintf(ofp, "  -u, --userspace [CMD]    Apply for the user-space tracer\n");
+	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 for the user-space tracer\n");
+	fprintf(ofp, "  -u, --userspace          Apply to the user-space tracer\n");
 #endif
 	fprintf(ofp, "\n");
 	fprintf(ofp, "Channel options:\n");
@@ -100,17 +100,17 @@
 		DEFAULT_CHANNEL_OVERWRITE ? "" : " (default)");
 	fprintf(ofp, "      --overwrite          Flight recorder mode%s\n",
 		DEFAULT_CHANNEL_OVERWRITE ? " (default)" : "");
-	fprintf(ofp, "      --subbuf-size        Subbuffer size in bytes\n");
+	fprintf(ofp, "      --subbuf-size SIZE   Subbuffer size in bytes\n");
 	fprintf(ofp, "                               (default: %u, kernel default: %u)\n",
 		DEFAULT_CHANNEL_SUBBUF_SIZE,
 		DEFAULT_KERNEL_CHANNEL_SUBBUF_SIZE);
-	fprintf(ofp, "      --num-subbuf         Number of subbufers\n");
+	fprintf(ofp, "      --num-subbuf NUM     Number of subbufers\n");
 	fprintf(ofp, "                               (default: %u, kernel default: %u)\n",
 		DEFAULT_CHANNEL_SUBBUF_NUM,
 		DEFAULT_KERNEL_CHANNEL_SUBBUF_NUM);
-	fprintf(ofp, "      --switch-timer       Switch timer interval in usec (default: %u)\n",
+	fprintf(ofp, "      --switch-timer USEC  Switch timer interval in usec (default: %u)\n",
 		DEFAULT_CHANNEL_SWITCH_TIMER);
-	fprintf(ofp, "      --read-timer         Read timer interval in usec (default: %u)\n",
+	fprintf(ofp, "      --read-timer USEC    Read timer interval in usec (default: %u)\n",
 		DEFAULT_CHANNEL_READ_TIMER);
 	fprintf(ofp, "\n");
 }
@@ -149,12 +149,14 @@
 /*
  * Adding channel using the lttng API.
  */
+/* TODO Rename as enable_channels() to follow pattern of disable_channels.c */
 static int enable_channel(char *session_name)
 {
 	int ret = CMD_SUCCESS;
 	char *channel_name;
 	struct lttng_domain dom;
 
+	/* TODO Enforce requirement of exactly one domain in cmd_...() */
 	/* Create lttng domain */
 	if (opt_kernel) {
 		dom.type = LTTNG_DOMAIN_KERNEL;
@@ -168,6 +170,7 @@
 
 	set_default_attr(&dom);
 
+	/* TODO Document and enforce return values (e.g. -1 should be CMD_FATAL) */
 	handle = lttng_create_handle(session_name, &dom);
 	if (handle == NULL) {
 		ret = -1;
@@ -176,8 +179,11 @@
 
 	/* Strip channel list (format: chan1,chan2,...) */
 	channel_name = strtok(opt_channels, ",");
+	/* TODO Issue CMD_WARNING if some lttng_enable_channel() calls fail */
+	/* TODO or if opt_channels is effectively empty */
 	while (channel_name != NULL) {
 		/* Copy channel name and normalize it */
+		/* TODO Use LTTNG_SYMBOL_NAME_LEN instead of NAME_MAX */
 		strncpy(chan.name, channel_name, NAME_MAX);
 		chan.name[NAME_MAX - 1] = '\0';
 
@@ -185,6 +191,7 @@
 
 		ret = lttng_enable_channel(handle, &chan);
 		if (ret < 0) {
+			/* TODO Issue ERR() here */
 			goto error;
 		} else {
 			MSG("%s channel %s enabled for session %s",
@@ -295,6 +302,7 @@
 	ret = enable_channel(session_name);
 
 end:
+	/* TODO Fix memory leak of session_name when opt_session_name == NULL */
 	poptFreeContext(pc);
 	return ret;
 }
diff --git a/src/bin/lttng/commands/enable_events.c b/src/bin/lttng/commands/enable_events.c
index f8bbc93..ad12fd2 100644
--- a/src/bin/lttng/commands/enable_events.c
+++ b/src/bin/lttng/commands/enable_events.c
@@ -98,17 +98,17 @@
 	fprintf(ofp, "\n");
 	fprintf(ofp, "  -h, --help               Show this help\n");
 	fprintf(ofp, "      --list-options       Simple listing of options\n");
-	fprintf(ofp, "  -s, --session            Apply on session name\n");
-	fprintf(ofp, "  -c, --channel            Apply on this channel\n");
+	fprintf(ofp, "  -s, --session NAME       Apply to session name\n");
+	fprintf(ofp, "  -c, --channel NAME       Apply to this channel\n");
 	fprintf(ofp, "  -a, --all                Enable all tracepoints\n");
-	fprintf(ofp, "  -k, --kernel             Apply for the kernel tracer\n");
+	fprintf(ofp, "  -k, --kernel             Apply to the kernel tracer\n");
 #if 0
-	fprintf(ofp, "  -u, --userspace [CMD]    Apply for the user-space tracer\n");
+	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 for the user-space tracer\n");
+	fprintf(ofp, "  -u, --userspace          Apply to the user-space tracer\n");
 #endif
 	fprintf(ofp, "\n");
 	fprintf(ofp, "Event options:\n");
@@ -138,6 +138,8 @@
 /*
  * Parse probe options.
  */
+/* TODO Document and enforce return values */
+/* TODO On success, returns 0; on error, returns -1 */
 static int parse_probe_opts(struct lttng_event *ev, char *opt)
 {
 	int ret;
@@ -222,6 +224,7 @@
 		channel_name = opt_channel_name;
 	}
 
+	/* TODO Enforce requirement of exactly one domain in cmd_...() */
 	if (opt_kernel && opt_userspace) {
 		ERR("Can't use -k/--kernel and -u/--userspace together");
 		ret = CMD_FATAL;
@@ -239,6 +242,7 @@
 		goto error;
 	}
 
+	/* TODO Document and enforce return values (e.g. -1 should be CMD_FATAL) */
 	handle = lttng_create_handle(session_name, &dom);
 	if (handle == NULL) {
 		ret = -1;
@@ -272,6 +276,7 @@
 						channel_name);
 			}
 			break;
+		/* TODO What of LTTNG_EVENT_SYSCALL in --userspace mode? */
 		case LTTNG_EVENT_ALL:
 			MSG("All %s events are enabled in channel %s",
 				opt_kernel ? "kernel" : "UST", channel_name);
@@ -281,6 +286,7 @@
 			 * We should not be here since lttng_enable_event should have
 			 * failed on the event type.
 			 */
+			/* TODO ret = CMD_FATAL; */
 			goto error;
 		}
 		goto end;
@@ -288,6 +294,8 @@
 
 	/* Strip event list */
 	event_name = strtok(opt_event_list, ",");
+	/* TODO Issue CMD_WARNING if some lttng_enable_event() calls fail */
+	/* TODO or if opt_event_list is effectively empty */
 	while (event_name != NULL) {
 		/* Copy name and type of the event */
 		strncpy(ev.name, event_name, LTTNG_SYMBOL_NAME_LEN);
diff --git a/src/bin/lttng/commands/list.c b/src/bin/lttng/commands/list.c
index d09ee57..68e7ee4 100644
--- a/src/bin/lttng/commands/list.c
+++ b/src/bin/lttng/commands/list.c
@@ -70,22 +70,22 @@
  */
 static void usage(FILE *ofp)
 {
-	fprintf(ofp, "usage: lttng list [SESSION [<OPTIONS>]]\n");
+	fprintf(ofp, "usage: lttng list [OPTIONS] [SESSION [SESSION_OPTIONS]]\n");
 	fprintf(ofp, "\n");
 	fprintf(ofp, "With no arguments, list available tracing session(s)\n");
 	fprintf(ofp, "\n");
-	fprintf(ofp, "With -k alone, list available kernel events\n");
-	fprintf(ofp, "With -u alone, list available userspace events\n");
+	fprintf(ofp, "Without a session, -k lists available kernel events\n");
+	fprintf(ofp, "Without a session, -u lists available userspace events\n");
 	fprintf(ofp, "\n");
 	fprintf(ofp, "  -h, --help              Show this help\n");
-	fprintf(ofp, "      --list-options       Simple listing of options\n");
+	fprintf(ofp, "      --list-options      Simple listing of options\n");
 	fprintf(ofp, "  -k, --kernel            Select kernel domain\n");
 	fprintf(ofp, "  -u, --userspace         Select user-space domain.\n");
 #if 0
 	fprintf(ofp, "  -p, --pid PID           List user-space events by PID\n");
 #endif
 	fprintf(ofp, "\n");
-	fprintf(ofp, "Options:\n");
+	fprintf(ofp, "Session Options:\n");
 	fprintf(ofp, "  -c, --channel NAME      List details of a channel\n");
 	fprintf(ofp, "  -d, --domain            List available domain(s)\n");
 	fprintf(ofp, "\n");
------------------------------

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



More information about the lttng-dev mailing list