[lttng-dev] [PATCH] enable_events and disable_events : Improve usage() printout, document and enforce return values, send --help to stdout, setup TODOs

Thibault, Daniel Daniel.Thibault at drdc-rddc.gc.ca
Tue Jan 31 17:28:41 EST 2012


>From 288de853ac784b52a6da3dff3918e88ac80c599e Tue, 31 Jan 2012 17:27:14 -0500
From: Daniel U. Thibault <daniel.thibault at drdc-rddc.gc.ca>
Date: Tue, 31 Jan 2012 17:26:57 -0500
Subject: [PATCH] enable_events and disable_events : Improve usage() printout, document and enforce return values, send --help to stdout, setup TODOs

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

diff --git a/src/bin/lttng/commands/disable_events.c b/src/bin/lttng/commands/disable_events.c
index 4d21f68..23f4858 100644
--- a/src/bin/lttng/commands/disable_events.c
+++ b/src/bin/lttng/commands/disable_events.c
@@ -74,25 +74,26 @@
 	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, "  -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, "                           (--kernel preempts --userspace)\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");
+	fprintf(ofp, "                           (--kernel preempts --userspace)\n");
 #endif
 	fprintf(ofp, "\n");
 }
 
 /*
- *  disable_events
- *
- *  Disabling event using the lttng API.
+ *  Disabling event(s) using the lttng API.
+ *  Returns a CMD_* result value.
  */
 static int disable_events(char *session_name)
 {
@@ -110,6 +111,7 @@
 		channel_name = opt_channel_name;
 	}
 
+	/* TODO: Either do this test everywhere or don't do it anywhere */
 	if (opt_kernel && opt_userspace) {
 		ERR("Can't use -k/--kernel and -u/--userspace together");
 		ret = CMD_FATAL;
@@ -129,13 +131,14 @@
 
 	handle = lttng_create_handle(session_name, &dom);
 	if (handle == NULL) {
-		ret = -1;
+		ret = CMD_ERROR;
 		goto error;
 	}
 
 	if (opt_disable_all) {
 		ret = lttng_disable_event(handle, NULL, channel_name);
 		if (ret < 0) {
+			ret = CMD_ERROR;
 			goto error;
 		}
 
@@ -146,6 +149,7 @@
 
 	/* Strip event list */
 	event_name = strtok(opt_event_list, ",");
+	/* TODO: Issue CMD_WARNING if one or more events fail, or if the event list is empty */
 	while (event_name != NULL) {
 		/* Kernel tracer action */
 		if (opt_kernel) {
@@ -163,11 +167,13 @@
 					event_name, channel_name);
 		} else {
 			ERR("Please specify a tracer (-k/--kernel or -u/--userspace)");
+			ret = CMD_ERROR;
 			goto error;
 		}
 
 		ret = lttng_disable_event(handle, event_name, channel_name);
 		if (ret < 0) {
+			ret = CMD_WARNING;
 			MSG("Unable to disable %s event %s in channel %s",
 					opt_kernel ? "kernel" : "UST", event_name,
 					channel_name);
@@ -192,9 +198,9 @@
 }
 
 /*
- *  cmd_disable_events
- *
- *  Disable event to trace session
+ *  Disable event(s) of trace session
+ *  An empty event list or misnamed events will yield a warning.
+ *  Returns a CMD_* result value.
  */
 int cmd_disable_events(int argc, const char **argv)
 {
@@ -208,7 +214,7 @@
 	while ((opt = poptGetNextOpt(pc)) != -1) {
 		switch (opt) {
 		case OPT_HELP:
-			usage(stderr);
+			usage(stdout);
 			ret = CMD_SUCCESS;
 			goto end;
 		case OPT_USERSPACE:
@@ -229,14 +235,14 @@
 	if (opt_event_list == NULL && opt_disable_all == 0) {
 		ERR("Missing event name(s).\n");
 		usage(stderr);
-		ret = CMD_SUCCESS;
+		ret = CMD_ERROR;
 		goto end;
 	}
 
 	if (!opt_session_name) {
 		session_name = get_session_name();
 		if (session_name == NULL) {
-			ret = -1;
+			ret = CMD_ERROR;
 			goto end;
 		}
 	} else {
@@ -246,6 +252,7 @@
 	ret = disable_events(session_name);
 
 end:
+	/* TODO: Free session_name when opt_session_name is NULL */
 	poptFreeContext(pc);
 	return ret;
 }
diff --git a/src/bin/lttng/commands/enable_events.c b/src/bin/lttng/commands/enable_events.c
index 01fae84..566ed61 100644
--- a/src/bin/lttng/commands/enable_events.c
+++ b/src/bin/lttng/commands/enable_events.c
@@ -98,17 +98,19 @@
 	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, "                           (--kernel preempts --userspace)\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");
+	fprintf(ofp, "                           (--kernel preempts --userspace)\n");
 #endif
 	fprintf(ofp, "\n");
 	fprintf(ofp, "Event options:\n");
@@ -137,6 +139,7 @@
 
 /*
  * Parse probe options.
+ * On success, returns 0; on error, returns -1
  */
 static int parse_probe_opts(struct lttng_event *ev, char *opt)
 {
@@ -163,6 +166,7 @@
 		ev->attr.probe.offset = strtoul(s_hex, NULL, 0);
 		DBG("probe offset %" PRIu64, ev->attr.probe.offset);
 		ev->attr.probe.addr = 0;
+		ret = 0; /* Success */
 		goto end;
 	}
 
@@ -176,6 +180,7 @@
 			ev->attr.probe.offset = 0;
 			DBG("probe offset %" PRIu64, ev->attr.probe.offset);
 			ev->attr.probe.addr = 0;
+			ret = 0; /* Success */
 			goto end;
 		}
 	}
@@ -192,6 +197,7 @@
 		DBG("probe addr %" PRIu64, ev->attr.probe.addr);
 		ev->attr.probe.offset = 0;
 		memset(ev->attr.probe.symbol_name, 0, LTTNG_SYMBOL_NAME_LEN);
+		ret = 0; /* Success */
 		goto end;
 	}
 
@@ -203,7 +209,8 @@
 }
 
 /*
- * Enabling event using the lttng API.
+ * Enabling event(s) using the lttng API.
+ * Returns a CMD_* result value.
  */
 static int enable_events(char *session_name)
 {
@@ -222,6 +229,7 @@
 		channel_name = opt_channel_name;
 	}
 
+	/* TODO: Either do this test everywhere or don't do it anywhere */
 	if (opt_kernel && opt_userspace) {
 		ERR("Can't use -k/--kernel and -u/--userspace together");
 		ret = CMD_FATAL;
@@ -241,7 +249,7 @@
 
 	handle = lttng_create_handle(session_name, &dom);
 	if (handle == NULL) {
-		ret = -1;
+		ret = CMD_ERROR;
 		goto error;
 	}
 
@@ -258,8 +266,10 @@
 
 		ret = lttng_enable_event(handle, &ev, channel_name);
 		if (ret < 0) {
+			ret = CMD_ERROR;
 			goto error;
 		}
+		ret = CMD_SUCCESS;
 
 		switch (opt_event_type) {
 		case LTTNG_EVENT_TRACEPOINT:
@@ -271,6 +281,7 @@
 				MSG("All kernel system calls are enabled in channel %s",
 						channel_name);
 			}
+			/* TODO: What of LTTNG_EVENT_SYSCALL in --userspace mode? */
 			break;
 		case LTTNG_EVENT_ALL:
 			MSG("All %s events are enabled in channel %s",
@@ -281,6 +292,7 @@
 			 * We should not be here since lttng_enable_event should have
 			 * failed on the event type.
 			 */
+			ret = CMD_FATAL;
 			goto error;
 		}
 		goto end;
@@ -288,6 +300,7 @@
 
 	/* Strip event list */
 	event_name = strtok(opt_event_list, ",");
+	/* TODO: Issue CMD_WARNING if one or more events fail, or if the event list is empty */
 	while (event_name != NULL) {
 		/* Copy name and type of the event */
 		strncpy(ev.name, event_name, LTTNG_SYMBOL_NAME_LEN);
@@ -309,7 +322,7 @@
 				ret = parse_probe_opts(&ev, opt_probe);
 				if (ret < 0) {
 					ERR("Unable to parse probe options");
-					ret = 0;
+					ret = CMD_ERROR;
 					goto error;
 				}
 				break;
@@ -317,7 +330,7 @@
 				ret = parse_probe_opts(&ev, opt_function);
 				if (ret < 0) {
 					ERR("Unable to parse function probe options");
-					ret = 0;
+					ret = CMD_ERROR;
 					goto error;
 				}
 				break;
@@ -370,6 +383,7 @@
 			}
 		} else {
 			ERR("Please specify a tracer (-k/--kernel or -u/--userspace)");
+			ret = CMD_ERROR;
 			goto error;
 		}
 
@@ -378,6 +392,7 @@
 			MSG("%s event %s created in channel %s",
 					opt_kernel ? "kernel": "UST", event_name, channel_name);
 		}
+		ret = CMD_SUCCESS;
 
 		/* Next event */
 		event_name = strtok(NULL, ",");
@@ -394,7 +409,8 @@
 }
 
 /*
- * Add event to trace session
+ * Add event(s) to trace session
+ * Returns a CMD_* result value.
  */
 int cmd_enable_events(int argc, const char **argv)
 {
@@ -411,7 +427,7 @@
 	while ((opt = poptGetNextOpt(pc)) != -1) {
 		switch (opt) {
 		case OPT_HELP:
-			usage(stderr);
+			usage(stdout);
 			ret = CMD_SUCCESS;
 			goto end;
 		case OPT_TRACEPOINT:
@@ -450,14 +466,14 @@
 	if (opt_event_list == NULL && opt_enable_all == 0) {
 		ERR("Missing event name(s).\n");
 		usage(stderr);
-		ret = CMD_SUCCESS;
+		ret = CMD_ERROR;
 		goto end;
 	}
 
 	if (!opt_session_name) {
 		session_name = get_session_name();
 		if (session_name == NULL) {
-			ret = -1;
+			ret = CMD_ERROR;
 			goto end;
 		}
 	} else {
------------------------------

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