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

Thibault, Daniel Daniel.Thibault at drdc-rddc.gc.ca
Thu Feb 2 14:59:19 EST 2012


>From 77d1ab08ea900802b5dc5c5585d92c09800d768b Thu, 2 Feb 2012 14:56:12 -0500
From: Daniel U. Thibault <daniel.thibault at drdc-rddc.gc.ca>
Date: Thu, 2 Feb 2012 14:55:45 -0500
Subject: [PATCH] enable_events.c : Document and enforce return values, detect strtoul() error, enforce exactly one domain flag, adjust usage() output accordingly, handle event list errors

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

diff --git a/src/bin/lttng/commands/enable_events.c b/src/bin/lttng/commands/enable_events.c
index db0597d..24cd286 100644
--- a/src/bin/lttng/commands/enable_events.c
+++ b/src/bin/lttng/commands/enable_events.c
@@ -21,6 +21,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <error.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
@@ -99,6 +100,7 @@
 static void usage(FILE *ofp)
 {
 	fprintf(ofp, "usage: lttng enable-event NAME[,NAME2,...] [options] [event_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");
@@ -167,6 +169,7 @@
 
 /*
  * Parse probe options.
+ * Returns a count of parsed options, or -1 in case of error.
  */
 static int parse_probe_opts(struct lttng_event *ev, char *opt)
 {
@@ -182,7 +185,7 @@
 	/* Check for symbol+offset */
 	ret = sscanf(opt, "%[^'+']+%s", name, s_hex);
 	if (ret == 2) {
-		strncpy(ev->attr.probe.symbol_name, name, LTTNG_SYMBOL_NAME_LEN);
+		strncpy(ev->attr.probe.symbol_name, name, LTTNG_SYMBOL_NAME_LEN-1);
 		ev->attr.probe.symbol_name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0';
 		DBG("probe symbol %s", ev->attr.probe.symbol_name);
 		if (strlen(s_hex) == 0) {
@@ -190,7 +193,13 @@
 			ret = -1;
 			goto end;
 		}
+		errno = 0;
 		ev->attr.probe.offset = strtoul(s_hex, NULL, 0);
+		if (errno != 0) {
+			ERR("Invalid probe offset value %s", s_hex);
+			ret = -1;
+			goto end;
+		}
 		DBG("probe offset %" PRIu64, ev->attr.probe.offset);
 		ev->attr.probe.addr = 0;
 		goto end;
@@ -200,7 +209,7 @@
 	if (isalpha(name[0])) {
 		ret = sscanf(opt, "%s", name);
 		if (ret == 1) {
-			strncpy(ev->attr.probe.symbol_name, name, LTTNG_SYMBOL_NAME_LEN);
+			strncpy(ev->attr.probe.symbol_name, name, LTTNG_SYMBOL_NAME_LEN-1);
 			ev->attr.probe.symbol_name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0';
 			DBG("probe symbol %s", ev->attr.probe.symbol_name);
 			ev->attr.probe.offset = 0;
@@ -218,7 +227,13 @@
 			ret = -1;
 			goto end;
 		}
+		errno = 0;
 		ev->attr.probe.addr = strtoul(s_hex, NULL, 0);
+		if (errno != 0) {
+			ERR("Invalid probe address value %s", s_hex);
+			ret = -1;
+			goto end;
+		}
 		DBG("probe addr %" PRIu64, ev->attr.probe.addr);
 		ev->attr.probe.offset = 0;
 		memset(ev->attr.probe.symbol_name, 0, LTTNG_SYMBOL_NAME_LEN);
@@ -233,7 +248,8 @@
 }
 
 /*
- * Maps loglevel from string to value
+ * Maps loglevel from string to value (string must not be NULL).
+ * Returns a TRACE_* enum value (0+), or -1 in case of error.
  */
 static
 int loglevel_str_to_value(const char *str)
@@ -277,6 +293,7 @@
 
 /*
  * Enabling event using the lttng API.
+ * Returns a CMD_* result value.
  */
 static int enable_events(char *session_name)
 {
@@ -286,14 +303,12 @@
 	struct lttng_domain dom;
 
 	/* Create lttng domain */
+	/* cmd_add_context() enforces the existence of exactly 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 */
 	}
 
 	if (opt_channel_name == NULL) {
@@ -306,9 +321,10 @@
 		channel_name = opt_channel_name;
 	}
 
+	handle = NULL;
 	handle = lttng_create_handle(session_name, &dom);
 	if (handle == NULL) {
-		ret = -1;
+		ret = CMD_FATAL;
 		goto error;
 	}
 
@@ -327,7 +343,7 @@
 				ev.loglevel = loglevel_str_to_value(opt_loglevel);
 				if (ev.loglevel == -1) {
 					ERR("Unknown loglevel %s", opt_loglevel);
-					ret = -1;
+					ret = CMD_ERROR;
 					goto error;
 				}
 			}
@@ -335,6 +351,7 @@
 
 		ret = lttng_enable_event(handle, &ev, channel_name);
 		if (ret < 0) {
+			ret = CMD_ERROR;
 			goto error;
 		}
 
@@ -360,6 +377,7 @@
 			 * We should not be here since lttng_enable_event should have
 			 * failed on the event type.
 			 */
+			ret = CMD_FATAL;
 			goto error;
 		}
 		goto end;
@@ -367,9 +385,11 @@
 
 	/* Strip event list */
 	event_name = strtok(opt_event_list, ",");
+	/* Default to a warning in case opt_event_list is empty */
+	ret = CMD_WARNING;
 	while (event_name != NULL) {
 		/* Copy name and type of the event */
-		strncpy(ev.name, event_name, LTTNG_SYMBOL_NAME_LEN);
+		strncpy(ev.name, event_name, LTTNG_SYMBOL_NAME_LEN-1);
 		ev.name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0';
 		ev.type = opt_event_type;
 
@@ -388,7 +408,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;
@@ -396,13 +416,13 @@
 				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;
 			case LTTNG_EVENT_FUNCTION_ENTRY:
 				strncpy(ev.attr.ftrace.symbol_name, opt_function_entry_symbol,
-						LTTNG_SYMBOL_NAME_LEN);
+						LTTNG_SYMBOL_NAME_LEN-1);
 				ev.attr.ftrace.symbol_name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0';
 				break;
 			case LTTNG_EVENT_SYSCALL:
@@ -414,7 +434,7 @@
 			}
 
 			if (opt_loglevel) {
-				MSG("Kernel loglevels are not supported.");
+				ERR("Kernel loglevels are not supported.");
 				ret = CMD_UNDEFINED;
 				goto error;
 			}
@@ -424,7 +444,7 @@
 		} else if (opt_userspace) {		/* User-space tracer action */
 #if 0
 			if (opt_cmd_name != NULL || opt_pid) {
-				MSG("Only supporting tracing all UST processes (-u) for now.");
+				ERR("Only supporting tracing all UST processes (-u) for now.");
 				ret = CMD_UNDEFINED;
 				goto error;
 			}
@@ -439,13 +459,17 @@
 			case LTTNG_EVENT_TRACEPOINT:
 				/* Copy name and type of the event */
 				ev.type = LTTNG_EVENT_TRACEPOINT;
-				strncpy(ev.name, event_name, LTTNG_SYMBOL_NAME_LEN);
+				strncpy(ev.name, event_name, LTTNG_SYMBOL_NAME_LEN-1);
 				ev.name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0';
 				break;
 			case LTTNG_EVENT_PROBE:
+				/* Fall-through */
 			case LTTNG_EVENT_FUNCTION:
+				/* Fall-through */
 			case LTTNG_EVENT_FUNCTION_ENTRY:
+				/* Fall-through */
 			case LTTNG_EVENT_SYSCALL:
+				/* Fall-through */
 			default:
 				ERR("Event type not available for user-space tracing");
 				ret = CMD_UNDEFINED;
@@ -457,14 +481,10 @@
 				ev.loglevel = loglevel_str_to_value(opt_loglevel);
 				if (ev.loglevel == -1) {
 					ERR("Unknown loglevel %s", opt_loglevel);
-					ret = -1;
+					ret = CMD_ERROR;
 					goto error;
 				}
 			}
-		} else {
-			ERR("Please specify a tracer (-k/--kernel or -u/--userspace)");
-			ret = CMD_ERROR;
-			goto error;
 		}
 
 		ret = lttng_enable_event(handle, &ev, channel_name);
@@ -475,10 +495,15 @@
 		} else {
 			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, ",");
+	}
+	if (ret == CMD_WARNING) {
+		ERR("No event specified for %s channel %s",
+				(opt_kernel ? "kernel" : "UST"), channel_name);
 	}
 
 end:
@@ -496,6 +521,7 @@
 
 /*
  * Add event to trace session
+ * Returns a CMD_* result value.
  */
 int cmd_enable_events(int argc, const char **argv)
 {
@@ -550,6 +576,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_event_list = (char*) poptGetArg(pc);
 	if (opt_event_list == NULL && opt_enable_all == 0) {
 		ERR("Missing event name(s).\n");
@@ -558,14 +596,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_events(session_name);
------------------------------

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