[lttng-dev] [PATCH] Harmonise lttng.h and lttng-ctl.c documentation

[lttng-dev] [PATCH] Harmonise lttng.h and lttng-ctl.c documentation

[Thibault, Daniel]

From e7b7ab737131c86540ff9f7ef1373e1a744d3bcc Tue, 31 Jan 2012 12:29:39 -0500
From: Daniel U. Thibault <daniel.thibault@drdc-rddc.gc.ca>
Date: Tue, 31 Jan 2012 12:29:17 -0500
Subject: [PATCH] Harmonise lttng.h and lttng-ctl.c documentation

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

diff --git a/include/lttng/lttng.h b/include/lttng/lttng.h
index 0fd323c..f402b5e 100644
--- a/include/lttng/lttng.h
+++ b/include/lttng/lttng.h
@@ -38,7 +38,7 @@
  */
 
 /*
- * Domain type are the different possible tracers.
+ * Domain types: the different possible tracers.
  */
 enum lttng_domain_type {
 	LTTNG_DOMAIN_KERNEL                   = 1,
@@ -205,104 +205,115 @@
 /*
  * Public LTTng control API
  *
- * For functions having a lttng domain type as parameter, if a bad value is
+ * For functions having an lttng domain type as parameter, if a bad value is
  * given, NO default is applied and an error is returned.
  *
  * On success, all functions of the API return 0 or the size of the allocated
- * array.
+ * array (in bytes).
  *
  * On error, a negative value is returned being a specific lttng-tools error
  * code which can be humanly interpreted with lttng_strerror(err).
+ *
+ * Exceptions to this are noted below.
  */
 
 /*
- * Create an handle used as a context for every request made to the library.
+ * Create a handle used as a context for every request made to the library.
  *
  * This handle contains the session name and lttng domain on which the command
- * will be executed on.
+ * will be executed.
+ * The returned pointer will be NULL in case of malloc() error.
  */
 extern struct lttng_handle *lttng_create_handle(const char *session_name,
 		struct lttng_domain *domain);
 
 /*
- * Destroy an handle. This will simply free(3) the data pointer returned by
- * lttng_create_handle() and rendering it unsuable.
+ * Destroy a handle. This will simply free(3) the data pointer returned by
+ * lttng_create_handle(), rendering it unusable.
  */
 extern void lttng_destroy_handle(struct lttng_handle *handle);
 
 /*
- * Create tracing session using a name and a path where trace will be written.
+ * Create a tracing session using a name and
+ * a path where the trace will be written.
  */
 extern int lttng_create_session(const char *name, const char *path);
 
 /*
- * Destroy tracing session.
+ * Destroy a tracing session.
  *
- * The session will not be useable anymore, tracing will stopped for all
- * registered trace and tracing buffers will be flushed.
+ * The session will not be usable anymore, tracing will be stopped for all
+ * registered traces, and the tracing buffers will be flushed.
  */
 extern int lttng_destroy_session(const char *name);
 
 /*
- * List all tracing sessions.
+ * List all the tracing sessions.
  *
- * Return the size of the "lttng_session" array. Caller must free(3).
+ * Return the size (number of entries) of the "lttng_session" array.
+ * Caller must free(3).
  */
 extern int lttng_list_sessions(struct lttng_session **sessions);
 
 /*
- * List registered domain(s) of a session.
+ * List the registered domain(s) of a session.
  *
- * Return the size of the "lttng_domain" array. Caller must free(3).
+ * Return the size (number of entries) of the "lttng_domain" array.
+ * Caller must free(3).
  */
 extern int lttng_list_domains(const char *session_name,
 		struct lttng_domain **domains);
 
 /*
- * List channel(s) of a session.
+ * List the channel(s) of a session.
  *
- * Return the size of the "lttng_channel" array. Caller must free(3).
+ * Return the size (number of entries) of the "lttng_channel" array.
+ * Caller must free(3).
  */
 extern int lttng_list_channels(struct lttng_handle *handle,
 		struct lttng_channel **channels);
 
 /*
- * List event(s) of a session channel.
+ * List the event(s) of a session channel.
  *
- * Return the size of the "lttng_event" array. Caller must free(3).
+ * Return the size (number of entries) of the "lttng_event" array.
+ * Caller must free(3).
  */
 extern int lttng_list_events(struct lttng_handle *handle,
 		const char *channel_name, struct lttng_event **events);
 
 /*
- * List available tracepoints of a specific lttng domain.
+ * List the available tracepoints of a specific lttng domain.
  *
- * Return the size of the "lttng_event" array. Caller must free(3).
+ * Return the size (number of entries) of the "lttng_event" array.
+ * Caller must free(3).
  */
 extern int lttng_list_tracepoints(struct lttng_handle *handle,
 		struct lttng_event **events);
 
 /*
  * Check if a session daemon is alive.
+ * Return 1 if alive or 0 if not. On error returns a negative value.
  */
 extern int lttng_session_daemon_alive(void);
 
 /*
- * Set tracing group for the *current* flow of execution.
+ * Set the tracing group for the *current* flow of execution.
+ * On success, returns 0, on error, returns -1 (null name) or -ENOMEM.
  */
 extern int lttng_set_tracing_group(const char *name);
 
 /*
- * Return a human readable error message of a lttng-tools error code.
+ * Return a human-readable error message for an lttng-tools error code.
  *
  * Parameter MUST be a negative value or else you'll get a generic message.
  */
 extern const char *lttng_strerror(int code);
 
 /*
- * This call permits to register an "outside consumer" to a session and a lttng
- * domain. No consumer will be spawned and all fds/commands will go through the
- * socket path given (socket_path).
+ * This call registers an "outside consumer" for a session and an lttng
+ * domain. No consumer will be spawned and all fds/commands will go through
+ * the socket path given (socket_path).
  *
  * NOTE: At the moment, if you use the liblttng-kconsumer, you can only use the
  * command socket. The error socket is not supported yet for roaming consumers.
@@ -311,32 +322,32 @@
 		const char *socket_path);
 
 /*
- * Start tracing for *all* registered trace (kernel and user-space).
+ * Start tracing for *all* registered traces (kernel and user-space).
  */
 extern int lttng_start_tracing(const char *session_name);
 
 /*
- * Stop tracing for *all* registered trace (kernel and user-space).
+ * Stop tracing for *all* registered traces (kernel and user-space).
  */
 extern int lttng_stop_tracing(const char *session_name);
 
 /*
- * Add context to event for a specific channel.
+ * Add context to event(s) for a specific channel (or for all).
  *
- * If event_name is NULL, the context is applied to all event of the channel.
+ * If event_name is NULL, the context is applied to all events of the channel.
  * If channel_name is NULL, a lookup of the event's channel is done.
- * If both are NULL, the context is applied on all events of all channels.
+ * If both are NULL, the context is applied to all events of all channels.
  */
 extern int lttng_add_context(struct lttng_handle *handle,
 		struct lttng_event_context *ctx, const char *event_name,
 		const char *channel_name);
 
 /*
- * Create or enable a kernel event.
+ * Create or enable a kernel event (or events) for a channel.
  *
  * If the event you are trying to enable does not exist, it will be created,
  * else it is enabled.
- *
+ * If event_name is NULL, all events are enabled.
  * If channel_name is NULL, the default channel is used (channel0).
  */
 extern int lttng_enable_event(struct lttng_handle *handle,
@@ -344,15 +355,14 @@
 
 /*
  * Create or enable a kernel channel.
- *
- * If name is NULL, the default channel is enabled (channel0).
+ * The channel name cannot be NULL.
  */
 extern int lttng_enable_channel(struct lttng_handle *handle,
 		struct lttng_channel *chan);
 
 /*
- * Disable kernel event.
- *
+ * Disable kernel event(s) of a channel and domain.
+ * If event_name is NULL, all events are disabled.
  * If channel_name is NULL, the default channel is used (channel0).
  */
 extern int lttng_disable_event(struct lttng_handle *handle,
@@ -360,8 +370,7 @@
 
 /*
  * Disable kernel channel.
- *
- * If channel_name is NULL, the default channel is disabled (channel0).
+ * The channel name cannot be NULL.
  */
 extern int lttng_disable_channel(struct lttng_handle *handle,
 		const char *name);
@@ -375,6 +384,7 @@
 /*
  * Set the default channel attributes for a specific domain and an allocated
  * lttng_channel_attr pointer.
+ * If either or both of the arguments are NULL, nothing happens.
  */
 extern void lttng_channel_set_default_attr(struct lttng_domain *domain,
 		struct lttng_channel_attr *attr);
diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c
index dfaa47f..1d92fac 100644
--- a/src/lib/lttng-ctl/lttng-ctl.c
+++ b/src/lib/lttng-ctl/lttng-ctl.c
@@ -466,7 +466,10 @@
 }
 
 /*
- * Add context to event or/and channel.
+ * Add context to event and/or channel.
+ * If event_name is NULL, the context is applied to all events of the channel.
+ * If channel_name is NULL, a lookup of the event's channel is done.
+ * If both are NULL, the context is applied to all events of all channels.
  *
  * Returns the size of the returned payload data or a negative error code.
  */
------------------------------

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

[lttng-dev] [PATCH] add_context.c : Document return values, simplify session_name assignment in cmd_add_context()

[Thibault, Daniel]

From 46d00e14190d536f428c25c0e969dde051845a48 Tue, 31 Jan 2012 14:02:51 -0500
From: Daniel U. Thibault <daniel.thibault@drdc-rddc.gc.ca>
Date: Tue, 31 Jan 2012 14:02:00 -0500
Subject: [PATCH] add_context.c : Document return values, simplify session_name assignment in cmd_add_context()

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 ce65024..2f6f11c 100644
--- a/src/bin/lttng/commands/add_context.c
+++ b/src/bin/lttng/commands/add_context.c
@@ -340,7 +340,8 @@
 }
 
 /*
- * Find context numerical value from string.
+ * Find context numerical value from string (which must not be NULL).
+ * Returns -1 if not found.
  */
 static int find_ctx_type_idx(const char *opt)
 {
@@ -360,6 +361,7 @@
 
 /*
  * Add context to channel or event.
+ * Returns a CMD_* result.
  */
 static int add_context(char *session_name)
 {
@@ -445,6 +447,7 @@
 
 /*
  * Add context to channel or event.
+ * Returns one of the CMD_* result values.
  */
 int cmd_add_context(int argc, const char **argv)
 {
@@ -514,14 +517,10 @@
 		}
 	}
 
-	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 = add_context(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/>

[lttng-dev] [PATCH] create.c : Fix memory leak

[Thibault, Daniel]

From e85aacb4c42fc522fce58971abc85ed2fed70123 Tue, 31 Jan 2012 14:31:56 -0500
From: Daniel U. Thibault <daniel.thibault@drdc-rddc.gc.ca>
Date: Tue, 31 Jan 2012 14:31:47 -0500
Subject: [PATCH] create.c : Fix memory leak

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

diff --git a/src/bin/lttng/commands/create.c b/src/bin/lttng/commands/create.c
index 99e5daa..e521024 100644
--- a/src/bin/lttng/commands/create.c
+++ b/src/bin/lttng/commands/create.c
@@ -132,6 +132,10 @@
 	ret = CMD_SUCCESS;
 
 error:
+	if (opt_session_name == NULL) {
+		free(session_name);
+	}
+
 	if (alloc_path) {
 		free(alloc_path);
 	}
------------------------------

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

[lttng-dev] [PATCH] disable_channels.c improvements and fixes

[Thibault, Daniel]

From 21f0eb1a7db1704645554d2f273bf80235be8240 Tue, 31 Jan 2012 15:13:29 -0500
From: Daniel U. Thibault <daniel.thibault@drdc-rddc.gc.ca>
Date: Tue, 31 Jan 2012 15:13:19 -0500
Subject: [PATCH] disable_channels.c improvements and fixes * Improve usage() printout * Document and enforce return values * Send --help to stdout * Issue CMD_WARNING for bad channel name(s) or an empty list of channel names * Fix a memory leak

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

diff --git a/src/bin/lttng/commands/disable_channels.c b/src/bin/lttng/commands/disable_channels.c
index bdbb657..4202963 100644
--- a/src/bin/lttng/commands/disable_channels.c
+++ b/src/bin/lttng/commands/disable_channels.c
@@ -70,25 +70,29 @@
 	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            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, "                           (--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");
 }
 
 /*
- * Disabling channel using the lttng API.
+ * Disabling channel(s) using the lttng API.
+ * session_name must not be NULL.
+ * Returns one of the CMD_* result values.
  */
 static int disable_channels(char *session_name)
 {
-	int ret = CMD_SUCCESS;
+	int warn = 0, ret;
 	char *channel_name;
 	struct lttng_domain dom;
 
@@ -105,19 +109,25 @@
 
 	handle = lttng_create_handle(session_name, &dom);
 	if (handle == NULL) {
-		ret = -1;
+		ret = CMD_ERROR;
 		goto error;
 	}
 
 	/* Strip channel list */
 	channel_name = strtok(opt_channels, ",");
+	/* Default to a warning in case opt_channels is empty */
+	ret = CMD_WARNING;
 	while (channel_name != NULL) {
 		DBG("Disabling channel %s", channel_name);
 
 		ret = lttng_disable_channel(handle, channel_name);
 		if (ret < 0) {
-			goto error;
+			warn = 1;
+			ERR("%s channel %s unrecognised for session %s",
+					opt_kernel ? "Kernel" : "UST", channel_name,
+					session_name);
 		} else {
+			ret = CMD_SUCCESS;
 			MSG("%s channel %s disabled for session %s",
 					opt_kernel ? "Kernel" : "UST", channel_name,
 					session_name);
@@ -125,6 +135,13 @@
 
 		/* Next channel */
 		channel_name = strtok(NULL, ",");
+	}
+	if (ret == CMD_WARNING) {
+		ERR("No %s channel specified for session %s",
+				opt_kernel ? "kernel" : "UST", session_name);
+	}
+	if (warn) {
+		ret = CMD_WARNING;
 	}
 
 error:
@@ -134,13 +151,13 @@
 }
 
 /*
- *  cmd_disable_channels
- *
- *  Disable channel to trace session
+ *  Disable channel(s) of a trace session.
+ *  Bad channel names or an empty list of channels will yield a warning.
+ *  Returns a CMD_* result value.
  */
 int cmd_disable_channels(int argc, const char **argv)
 {
-	int opt, ret;
+	int opt, ret = CMD_SUCCESS;
 	static poptContext pc;
 	char *session_name = NULL;
 
@@ -150,15 +167,13 @@
 	while ((opt = poptGetNextOpt(pc)) != -1) {
 		switch (opt) {
 		case OPT_HELP:
-			usage(stderr);
-			ret = CMD_SUCCESS;
+			usage(stdout);
 			goto end;
 		case OPT_USERSPACE:
 			opt_userspace = 1;
 			break;
 		case OPT_LIST_OPTIONS:
 			list_cmd_options(stdout, long_options);
-			ret = CMD_SUCCESS;
 			goto end;
 		default:
 			usage(stderr);
@@ -171,22 +186,23 @@
 	if (opt_channels == NULL) {
 		ERR("Missing channel 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;
-			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 cleanup;
 	}
 
 	ret = disable_channels(session_name);
 
+cleanup:
+	if (opt_session_name == NULL) {
+		free(session_name);
+	}
+
 end:
 	return ret;
 }
------------------------------

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

[lttng-dev] [PATCH] disable_channels.c and enable_channels.c

[Thibault, Daniel]

From c297ddf1295b316d1133a49bc8d2fa08f115dcdc Tue, 31 Jan 2012 16:32:47 -0500
From: Daniel U. Thibault <daniel.thibault@drdc-rddc.gc.ca>
Date: Tue, 31 Jan 2012 16:32:33 -0500
Subject: [PATCH] disable_channels.c : * Also use the enable_channels.c channel name length filter enable_channels.c : * Improve usage() printout * Document and enforce return values * Send --help to stdout * Return CMD_WARNING for bad channel name(s) * Fix session_name memory leak * Use LTTNG_SYMBOL_NAME_LEN instead of NAME_MAX for chan.name * Replace atol/atoi with strtol (because the former's behaviour is undefined for bad input) (this required including errno.h) * Change private function enable_channel() to enable_channels() to be consistent with disable_channels.c

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

diff --git a/src/bin/lttng/commands/disable_channels.c b/src/bin/lttng/commands/disable_channels.c
index 4202963..f6929e6 100644
--- a/src/bin/lttng/commands/disable_channels.c
+++ b/src/bin/lttng/commands/disable_channels.c
@@ -94,6 +94,8 @@
 {
 	int warn = 0, ret;
 	char *channel_name;
+	char chan_name[LTTNG_SYMBOL_NAME_LEN];
+
 	struct lttng_domain dom;
 
 	/* Create lttng domain */
@@ -120,16 +122,20 @@
 	while (channel_name != NULL) {
 		DBG("Disabling channel %s", channel_name);
 
-		ret = lttng_disable_channel(handle, channel_name);
+		/* Copy channel name and normalize it */
+		strncpy(chan_name, channel_name, LTTNG_SYMBOL_NAME_LEN - 1);
+		chan_name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0';
+
+		ret = lttng_disable_channel(handle, chan_name);
 		if (ret < 0) {
 			warn = 1;
 			ERR("%s channel %s unrecognised for session %s",
-					opt_kernel ? "Kernel" : "UST", channel_name,
+					opt_kernel ? "Kernel" : "UST", chan_name,
 					session_name);
 		} else {
 			ret = CMD_SUCCESS;
 			MSG("%s channel %s disabled for session %s",
-					opt_kernel ? "Kernel" : "UST", channel_name,
+					opt_kernel ? "Kernel" : "UST", chan_name,
 					session_name);
 		}
 
diff --git a/src/bin/lttng/commands/enable_channels.c b/src/bin/lttng/commands/enable_channels.c
index 03de524..7bebf76 100644
--- a/src/bin/lttng/commands/enable_channels.c
+++ b/src/bin/lttng/commands/enable_channels.c
@@ -21,6 +21,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <errno.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
@@ -84,15 +85,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, "  -k, --kernel             Apply on the kernel tracer\n");
+	fprintf(ofp, "  -s, --session            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, "                           (--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, "Channel options:\n");
@@ -100,7 +103,7 @@
 		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        Subbuffer size (in bytes)\n");
 	fprintf(ofp, "                               (default: %u, kernel default: %u)\n",
 		DEFAULT_CHANNEL_SUBBUF_SIZE,
 		DEFAULT_KERNEL_CHANNEL_SUBBUF_SIZE);
@@ -118,14 +121,16 @@
 /*
  * Set default attributes depending on those already defined from the command
  * line.
+ * The domain argument must not be NULL.
  */
 static void set_default_attr(struct lttng_domain *dom)
 {
 	struct lttng_channel_attr default_attr;
 
-	/* Set attributes */
+	/* Recover default attributes */
 	lttng_channel_set_default_attr(dom, &default_attr);
 
+	/* Set attributes */
 	if (chan.attr.overwrite == -1) {
 		chan.attr.overwrite = default_attr.overwrite;
 	}
@@ -147,11 +152,13 @@
 }
 
 /*
- * Adding channel using the lttng API.
+ * Enables channel(s) using the lttng API.
+ * Channel names which are too long will be truncated.
+ * Returns one of the CMD_* result values.
  */
-static int enable_channel(char *session_name)
+static int enable_channels(char *session_name)
 {
-	int ret = CMD_SUCCESS;
+	int warn = 0, ret;
 	char *channel_name;
 	struct lttng_domain dom;
 
@@ -170,23 +177,29 @@
 
 	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);
-		chan.name[NAME_MAX - 1] = '\0';
-
 		DBG("Enabling channel %s", channel_name);
+
+		/* Copy channel name and normalize it */
+		strncpy(chan.name, channel_name, LTTNG_SYMBOL_NAME_LEN - 1);
+		chan.name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0';
 
 		ret = lttng_enable_channel(handle, &chan);
 		if (ret < 0) {
-			goto error;
+			warn = 1;
+			ERR("%s channel %s unrecognised for session %s",
+					opt_kernel ? "Kernel" : "UST", channel_name,
+					session_name);
 		} else {
+			ret = CMD_SUCCESS;
 			MSG("%s channel %s enabled for session %s",
 					opt_kernel ? "Kernel" : "UST", channel_name,
 					session_name);
@@ -194,6 +207,13 @@
 
 		/* Next event */
 		channel_name = strtok(NULL, ",");
+	}
+	if (ret == CMD_WARNING) {
+		ERR("No %s channel specified for session %s",
+				opt_kernel ? "kernel" : "UST", session_name);
+	}
+	if (warn) {
+		ret = CMD_WARNING;
 	}
 
 error:
@@ -215,7 +235,8 @@
 }
 
 /*
- * Add channel to trace session
+ * Add channel(s) to trace session
+ * Returns one of the CMD_* result values.
  */
 int cmd_enable_channels(int argc, const char **argv)
 {
@@ -231,7 +252,7 @@
 	while ((opt = poptGetNextOpt(pc)) != -1) {
 		switch (opt) {
 		case OPT_HELP:
-			usage(stderr);
+			usage(stdout);
 			ret = CMD_SUCCESS;
 			goto end;
 		case OPT_DISCARD:
@@ -243,20 +264,44 @@
 			DBG("Channel set to overwrite");
 			break;
 		case OPT_SUBBUF_SIZE:
-			chan.attr.subbuf_size = atol(poptGetOptArg(pc));
+			errno = 0;
+			chan.attr.subbuf_size = strtol(poptGetOptArg(pc), (char **)NULL, 10);
+			if (errno != 0) {
+				ERR("Subbuffer size not a number");
+				ret = CMD_ERROR;
+				goto end;
+			}
 			DBG("Channel subbuf size set to %" PRIu64, chan.attr.subbuf_size);
 			break;
 		case OPT_NUM_SUBBUF:
-			chan.attr.num_subbuf = atoi(poptGetOptArg(pc));
+			errno = 0;
+			chan.attr.num_subbuf = strtol(poptGetOptArg(pc), (char **)NULL, 10);
+			if (errno != 0) {
+				ERR("Number of subbuffers not a number");
+				ret = CMD_ERROR;
+				goto end;
+			}
 			DBG("Channel subbuf num set to %" PRIu64, chan.attr.num_subbuf);
 			break;
 		case OPT_SWITCH_TIMER:
-			chan.attr.switch_timer_interval = atoi(poptGetOptArg(pc));
-			DBG("Channel switch timer interval set to %d", chan.attr.switch_timer_interval);
+			errno = 0;
+			chan.attr.switch_timer_interval = strtol(poptGetOptArg(pc), (char **)NULL, 10);
+			if (errno != 0) {
+				ERR("Switch timer interval not a number");
+				ret = CMD_ERROR;
+				goto end;
+			}
+			DBG("Channel switch timer interval set to %d usec", chan.attr.switch_timer_interval);
 			break;
 		case OPT_READ_TIMER:
-			chan.attr.read_timer_interval = atoi(poptGetOptArg(pc));
-			DBG("Channel read timer interval set to %d", chan.attr.read_timer_interval);
+			errno = 0;
+			chan.attr.read_timer_interval = strtol(poptGetOptArg(pc), (char **)NULL, 10);
+			if (errno != 0) {
+				ERR("Read timer interval not a number");
+				ret = CMD_ERROR;
+				goto end;
+			}
+			DBG("Channel read timer interval set to %d usec", chan.attr.read_timer_interval);
 			break;
 		case OPT_USERSPACE:
 			opt_userspace = 1;
@@ -274,24 +319,26 @@
 
 	opt_channels = (char*) poptGetArg(pc);
 	if (opt_channels == NULL) {
-		ERR("Missing channel name.\n");
+		ERR("Missing channel 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;
-			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 cleanup;
 	}
 
-	ret = enable_channel(session_name);
+	ret = enable_channels(session_name);
+
+cleanup:
+	if (opt_session_name == NULL) {
+		free(session_name);
+	}
 
 end:
	poptFreeContext(pc);
------------------------------

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