>From 1b5ba907a4910bd10dda3fbd14c1f1751893e268 Wed, 1 Feb 2012 17:48:14 -0500
From: Daniel U. Thibault <[email protected]>
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 <[email protected]>

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

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

Reply via email to