>From c297ddf1295b316d1133a49bc8d2fa08f115dcdc Tue, 31 Jan 2012 16:32:47 -0500 From: Daniel U. Thibault <[email protected]> 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 <[email protected]> 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/> _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
