-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 This one also does not apply...
Just rebase it over the new head and it will be a charm :). It's actually a good thing this one goes into stable so I'll wait for it. Thanks! David On 12-01-31 05:28 PM, Thibault, Daniel wrote: > From 288de853ac784b52a6da3dff3918e88ac80c599e Tue, 31 Jan 2012 17:27:14 -0500 > From: Daniel U. Thibault <[email protected]> > 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 <[email protected]> > > 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/> > > _______________________________________________ > lttng-dev mailing list > [email protected] > http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev > -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) iQEcBAEBAgAGBQJPKYElAAoJEELoaioR9I02YpEIALoF7HJ+X5WjkXKhhe/Txb7I rMm9PXLWmN+RHE9mqndw7VTmHjwapIzDcsu9ymmvxM8+vIRNcZw2jfjr/WUjtjVW bI7TjcP8fZjAzQVv/O1Sth7Pe4XPclnQNX2ZFMRxq6dqzkmgOF7oBGcKk2Z/tBrG JByG+Weac4EChegL1mw7fz5xsVrLQQVO4QrvH0lUYL6GQqwZEb00rJkde9RsKAXK D7WYKWBBPsBWcWkV/iDdHlRXUPvqmFGIi2QfFIV4gWpFntmX0MvVEwONO6Hr7yyl cUN34/JJN47vyB13ULYevdu3r8Bg+CeZaNr+ghXGCaxleyWzaNcPzSznKKfda10= =wM9l -----END PGP SIGNATURE----- _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
