>From 77d1ab08ea900802b5dc5c5585d92c09800d768b Thu, 2 Feb 2012 14:56:12 -0500
From: Daniel U. Thibault <[email protected]>
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 <[email protected]>

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

_______________________________________________
lttng-dev mailing list
[email protected]
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Reply via email to