From af87c45a6179026e263c3a9eb251ccf8ec9537e7 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 27 Jan 2012 10:37:45 -0500 Subject: [PATCH] Fix comments and enforce expected return values Improve calibrate and add_context usage() printout. Document and enforce the expected return values for the cmd_calibrate/cmd_addcontext chain of functions. Fix add_context usage() Add documentation in liblttng-ctl. Signed-off-by: Daniel Thibault Signed-off-by: David Goulet --- src/bin/lttng-sessiond/main.c | 6 +++--- src/bin/lttng/commands/add_context.c | 27 ++++++++++++++++++--------- src/bin/lttng/commands/calibrate.c | 25 +++++++++++++------------ src/bin/lttng/lttng.c | 19 ++++++++++--------- src/lib/lttng-ctl/lttng-ctl.c | 6 ++++-- 5 files changed, 48 insertions(+), 35 deletions(-) diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c index 4e5771f03..631aa6b2d 100644 --- a/src/bin/lttng-sessiond/main.c +++ b/src/bin/lttng-sessiond/main.c @@ -4077,14 +4077,14 @@ static void sighandler(int sig) { switch (sig) { case SIGPIPE: - DBG("SIGPIPE caugth"); + DBG("SIGPIPE caught"); return; case SIGINT: - DBG("SIGINT caugth"); + DBG("SIGINT caught"); stop_threads(); break; case SIGTERM: - DBG("SIGTERM caugth"); + DBG("SIGTERM caught"); stop_threads(); break; default: diff --git a/src/bin/lttng/commands/add_context.c b/src/bin/lttng/commands/add_context.c index 82abc7333..73aff3113 100644 --- a/src/bin/lttng/commands/add_context.c +++ b/src/bin/lttng/commands/add_context.c @@ -303,33 +303,35 @@ static void usage(FILE *ofp) { fprintf(ofp, "usage: lttng add-context -t TYPE\n"); fprintf(ofp, "\n"); - fprintf(ofp, "If no event name is given (-e), the context will be added to the channel\n"); fprintf(ofp, "If no channel and no event is given (-c/-e), the context\n"); fprintf(ofp, "will be added to all events and all channels.\n"); + fprintf(ofp, "Otherwise the context will be added only to the channel (-c)\n"); + fprintf(ofp, "and/or event (-e) indicated.\n"); fprintf(ofp, "\n"); fprintf(ofp, "Options:\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, " -s, --session NAME Apply on session name\n"); fprintf(ofp, " -c, --channel NAME Apply on channel\n"); fprintf(ofp, " -e, --event NAME Apply on event\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, " -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, " -t, --type TYPE Context type. You can repeat that option on\n"); - fprintf(ofp, " the command line.\n"); + fprintf(ofp, " the command line to specify multiple contexts at once.\n"); + fprintf(ofp, " (--kernel preempts --userspace)\n"); fprintf(ofp, " TYPE can be one of the strings below:\n"); print_ctx_type(ofp); fprintf(ofp, "\n"); fprintf(ofp, "Example:\n"); fprintf(ofp, "This command will add the context information 'prio' and two perf\n" - "counters: hardware branch misses and cache misses, to all events\n" + "counters (hardware branch misses and cache misses), to all events\n" "in the trace data output:\n"); fprintf(ofp, "# lttng add-context -k -t prio -t perf:branch-misses -t perf:cache-misses\n"); fprintf(ofp, "\n"); @@ -377,7 +379,7 @@ static int add_context(char *session_name) handle = lttng_create_handle(session_name, &dom); if (handle == NULL) { - ret = -1; + ret = CMD_ERROR; goto error; } @@ -401,7 +403,7 @@ static int add_context(char *session_name) ret = lttng_add_context(handle, &context, opt_event_name, opt_channel_name); if (ret < 0) { - fprintf(stderr, "%s: ", type->opt->symbol); + ERR("%s: ", type->opt->symbol); continue; } else { MSG("%s context %s added to %s event in %s", @@ -411,6 +413,8 @@ static int add_context(char *session_name) } } + ret = CMD_SUCCESS; + error: lttng_destroy_handle(handle); @@ -448,6 +452,11 @@ int cmd_add_context(int argc, const char **argv) ret = -1; goto end; } + + /* + * Look up the index of opt_type in ctx_opts[] first, so we don't + * have to free(type) on failure. + */ index = find_ctx_type_idx(opt_type); if (index < 0) { ERR("Unknown context type %s", opt_type); diff --git a/src/bin/lttng/commands/calibrate.c b/src/bin/lttng/commands/calibrate.c index 81eadbfd2..2c1adb16c 100644 --- a/src/bin/lttng/commands/calibrate.c +++ b/src/bin/lttng/commands/calibrate.c @@ -89,14 +89,14 @@ static void usage(FILE *ofp) fprintf(ofp, "\n"); fprintf(ofp, " -h, --help Show this help\n"); fprintf(ofp, " --list-options Simple listing of options\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, " -u, --userspace [CMD] Apply to the user-space tracer (domain: UST\n"); + fprintf(ofp, " EXEC_NAME). If no CMD, the domain is UST global.\n"; + fprintf(ofp, " (-k preempts -u)\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, "Calibrate options:\n"); @@ -117,9 +117,9 @@ static void usage(FILE *ofp) } /* - * calibrate_lttng + * Calibrate LTTng. * - * Calibrate LTTng. + * Returns a CMD_* error. */ static int calibrate_lttng(void) { @@ -140,7 +140,7 @@ static int calibrate_lttng(void) handle = lttng_create_handle(NULL, &dom); if (handle == NULL) { - ret = -1; + ret = CMD_ERROR; goto error; } @@ -156,6 +156,7 @@ static int calibrate_lttng(void) calibrate.type = LTTNG_CALIBRATE_FUNCTION; ret = lttng_calibrate(handle, &calibrate); if (ret < 0) { + ret = CMD_ERROR; goto error; } MSG("%s calibration done", opt_kernel ? "Kernel" : "UST"); @@ -171,6 +172,8 @@ static int calibrate_lttng(void) goto error; } + ret = CMD_SUCCESS; + error: lttng_destroy_handle(handle); @@ -178,9 +181,7 @@ error: } /* - * cmd_calibrate - * - * Calibrate LTTng tracer. + * Calibrate LTTng tracer. */ int cmd_calibrate(int argc, const char **argv) { @@ -196,7 +197,7 @@ int cmd_calibrate(int argc, const char **argv) while ((opt = poptGetNextOpt(pc)) != -1) { switch (opt) { case OPT_HELP: - usage(stderr); + usage(stdout); ret = CMD_SUCCESS; goto end; case OPT_TRACEPOINT: diff --git a/src/bin/lttng/lttng.c b/src/bin/lttng/lttng.c index e60b694e4..36d4c913d 100644 --- a/src/bin/lttng/lttng.c +++ b/src/bin/lttng/lttng.c @@ -316,19 +316,20 @@ static int spawn_sessiond(char *pathname) } else if (pid > 0) { sessiond_pid = pid; /* - * Wait for lttng-sessiond to start. We need to use a - * flag to check if the signal has been sent to us, - * because the child can be scheduled before the parent, - * and thus send the signal before this check. In the - * signal handler, we set the recv_child_signal flag, so - * anytime we check it after the fork is fine. Note that - * sleep() is interrupted before the 1 second delay as - * soon as the signal is received, so it will not cause - * visible delay for the user. + * Wait for lttng-sessiond to start. We need to use a flag to check if + * the signal has been sent to us, because the child can be scheduled + * before the parent, and thus send the signal before this check. In + * the signal handler, we set the recv_child_signal flag, so anytime we + * check it after the fork is fine. Note that sleep() is interrupted + * before the 1 second delay as soon as the signal is received, so it + * will not cause visible delay for the user. */ while (!recv_child_signal) { sleep(1); } + /* + * The signal handler will nullify sessiond_pid on SIGCHLD + */ if (!sessiond_pid) { exit(EXIT_FAILURE); } diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c index b7fc78161..6cc04efc9 100644 --- a/src/lib/lttng-ctl/lttng-ctl.c +++ b/src/lib/lttng-ctl/lttng-ctl.c @@ -302,7 +302,7 @@ static int disconnect_sessiond(void) /* * Ask the session daemon a specific command and put the data into buf. * - * Return size of data (only payload, not header). + * Return size of data (only payload, not header) or a negative error code. */ static int ask_sessiond(struct lttcomm_session_msg *lsm, void **buf) { @@ -462,6 +462,8 @@ int lttng_stop_tracing(const char *session_name) /* * Add context to event or/and channel. + * + * Returns the size of the returned payload data or a negative error code. */ int lttng_add_context(struct lttng_handle *handle, struct lttng_event_context *ctx, const char *event_name, @@ -807,7 +809,7 @@ int lttng_set_tracing_group(const char *name) } /* - * lttng_calibrate + * Returns size of returned session payload data or a negative error code. */ int lttng_calibrate(struct lttng_handle *handle, struct lttng_calibrate *calibrate) -- 2.34.1