[lttng-dev] [PATCH] enable_events and disable_events : Improve usage() printout, document and enforce return values, send --help to stdout, setup TODOs
David Goulet
dgoulet at efficios.com
Wed Feb 1 13:15:01 EST 2012
-----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 <daniel.thibault at drdc-rddc.gc.ca>
> 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 <daniel.thibault at drdc-rddc.gc.ca>
>
> 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
> lttng-dev at lists.lttng.org
> 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-----
More information about the lttng-dev
mailing list