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 <daniel.thibault@drdc-rddc.gc.ca>
Signed-off-by: David Goulet <dgoulet@efficios.com>

diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index 4e5771f03639b476748c97a459e8c7aab0829fee..631aa6b2d92e5e0d13aaa5a2d59072327b31eb69 100644 (file)
--- 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 82abc73332ad10b1117a7e95a954f39f4256108e..73aff31130e8e5e5f5489d988bff3583ae8ecc86 100644 (file)
--- 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 81eadbfd2b6d54499fd7d657e49a051361ac0c8b..2c1adb16c747d46ba2f7d2c27db1051ea94656b3 100644 (file)
--- 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 e60b694e43729ab1c7ced011e11c2cae3a5e5433..36d4c913df48ab681cbfac27e428c3f7888d339c 100644 (file)
--- 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 b7fc781610f9b214422d84bc5a9a34f97bb2b059..6cc04efc939359a139c900867a71778db776bd77 100644 (file)
--- 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)