* [lttng-dev] [PATCH] enable_events and disable_events : Improve usage() printout, document and enforce return values, send --help to stdout, setup TODOs
[not found] <mailman.123.1328045961.1530.lttng-dev@lists.lttng.org>
@ 2012-01-31 22:28 ` Thibault, Daniel
0 siblings, 0 replies; only message in thread
From: Thibault, Daniel @ 2012-01-31 22:28 UTC (permalink / raw)
From 288de853ac784b52a6da3dff3918e88ac80c599e Tue, 31 Jan 2012 17:27:14 -0500
From: Daniel U. Thibault <daniel.thibault@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/>
^ permalink raw reply [flat|nested] only message in thread