MINOR: trace: split the CLI "trace" parser in CLI vs statement
In order to be able to reuse the "trace" statements elsewhere (e.g.
global section), we'll first need to split its parser. It turns out
that the whole thing is self-contained inside a single function that
emits a single message on warning/error or nothing on success. That's
quite easy to split in two parts, the one that does the job and produces
the status message and the one that sends it to the CLI. That's what
this patch does.
diff --git a/src/trace.c b/src/trace.c
index 5909dd4..243174a 100644
--- a/src/trace.c
+++ b/src/trace.c
@@ -299,14 +299,21 @@
return NULL;
}
-/* parse the command, returns 1 if a message is returned, otherwise zero */
-static int cli_parse_trace(char **args, char *payload, struct appctx *appctx, void *private)
+/* Parse a "trace" statement. Returns a severity as a LOG_* level and a status
+ * message that may be delivered to the user, in <msg>. The message will be
+ * nulled first and msg must be a valid pointer. A null status message output
+ * indicates no error. Be careful not to use the return value as a boolean, as
+ * LOG_* values are not ordered as one could imagine (LOG_EMERG is zero). The
+ * function may/will use the trash buffer as the storage for the response
+ * message so that the caller never needs to release anything.
+ */
+static int trace_parse_statement(char **args, const char **msg)
{
struct trace_source *src;
uint64_t *ev_ptr = NULL;
- if (!cli_has_level(appctx, ACCESS_LVL_OPER))
- return 1;
+ /* no error by default */
+ *msg = NULL;
if (!*args[1]) {
/* no arg => report the list of supported sources as a warning */
@@ -319,33 +326,36 @@
chunk_appendf(&trash, " [%c] %-10s : %s\n", trace_state_char(src->state), src->name.ptr, src->desc);
trash.area[trash.data] = 0;
- return cli_msg(appctx, LOG_WARNING, trash.area);
+ *msg = trash.area;
+ return LOG_WARNING;
}
if (strcmp(args[1], "0") == 0) {
/* emergency stop of all traces */
list_for_each_entry(src, &trace_sources, source_link)
HA_ATOMIC_STORE(&src->state, TRACE_STATE_STOPPED);
- return cli_msg(appctx, LOG_NOTICE, "All traces now stopped");
+ *msg = "All traces now stopped";
+ return LOG_NOTICE;
}
src = trace_find_source(args[1]);
- if (!src)
- return cli_err(appctx, "No such trace source");
+ if (!src) {
+ *msg = "No such trace source";
+ return LOG_ERR;
+ }
if (!*args[2]) {
- return cli_msg(appctx, LOG_WARNING,
- "Supported commands:\n"
- " event : list/enable/disable source-specific event reporting\n"
- //" filter : list/enable/disable generic filters\n"
- " level : list/set trace reporting level\n"
- " lock : automatic lock on thread/connection/stream/...\n"
- " pause : pause and automatically restart after a specific event\n"
- " sink : list/set event sinks\n"
- " start : start immediately or after a specific event\n"
- " stop : stop immediately or after a specific event\n"
- " verbosity : list/set trace output verbosity\n"
- );
+ *msg = "Supported commands:\n"
+ " event : list/enable/disable source-specific event reporting\n"
+ //" filter : list/enable/disable generic filters\n"
+ " level : list/set trace reporting level\n"
+ " lock : automatic lock on thread/connection/stream/...\n"
+ " pause : pause and automatically restart after a specific event\n"
+ " sink : list/set event sinks\n"
+ " start : start immediately or after a specific event\n"
+ " stop : stop immediately or after a specific event\n"
+ " verbosity : list/set trace output verbosity\n";
+ return LOG_WARNING;
}
else if ((strcmp(args[2], "event") == 0 && (ev_ptr = &src->report_events)) ||
(strcmp(args[2], "pause") == 0 && (ev_ptr = &src->pause_events)) ||
@@ -379,7 +389,8 @@
src->known_events[i].name, src->known_events[i].desc);
}
trash.area[trash.data] = 0;
- return cli_msg(appctx, LOG_WARNING, trash.area);
+ *msg = trash.area;
+ return LOG_WARNING;
}
if (strcmp(name, "now") == 0 && ev_ptr != &src->report_events) {
@@ -395,6 +406,7 @@
HA_ATOMIC_STORE(&src->lockon_ptr, NULL);
HA_ATOMIC_STORE(&src->state, TRACE_STATE_STOPPED);
}
+ *msg = NULL;
return 0;
}
@@ -404,8 +416,10 @@
HA_ATOMIC_STORE(ev_ptr, ~0);
else {
ev = trace_find_event(src->known_events, name);
- if (!ev)
- return cli_err(appctx, "No such trace event");
+ if (!ev) {
+ *msg = "No such trace event";
+ return LOG_ERR;
+ }
if (!neg)
HA_ATOMIC_OR(ev_ptr, ev->mask);
@@ -426,15 +440,18 @@
sink->name, sink->desc);
}
trash.area[trash.data] = 0;
- return cli_msg(appctx, LOG_WARNING, trash.area);
+ *msg = trash.area;
+ return LOG_WARNING;
}
if (strcmp(name, "none") == 0)
sink = NULL;
else {
sink = sink_find(name);
- if (!sink)
- return cli_err(appctx, "No such sink");
+ if (!sink) {
+ *msg = "No such sink";
+ return LOG_ERR;
+ }
}
HA_ATOMIC_STORE(&src->sink, sink);
@@ -457,7 +474,8 @@
chunk_appendf(&trash, " %c developer : also report information useful only to the developer\n",
src->level == TRACE_LEVEL_DEVELOPER ? '*' : ' ');
trash.area[trash.data] = 0;
- return cli_msg(appctx, LOG_WARNING, trash.area);
+ *msg = trash.area;
+ return LOG_WARNING;
}
if (strcmp(name, "error") == 0)
@@ -472,8 +490,10 @@
HA_ATOMIC_STORE(&src->level, TRACE_LEVEL_DATA);
else if (strcmp(name, "developer") == 0)
HA_ATOMIC_STORE(&src->level, TRACE_LEVEL_DEVELOPER);
- else
- return cli_err(appctx, "No such trace level");
+ else {
+ *msg = "No such trace level";
+ return LOG_ERR;
+ }
}
else if (strcmp(args[2], "lock") == 0) {
const char *name = args[3];
@@ -539,7 +559,8 @@
src->lockon_args[3].name, src->lockon_args[3].desc);
trash.area[trash.data] = 0;
- return cli_msg(appctx, LOG_WARNING, trash.area);
+ *msg = trash.area;
+ return LOG_WARNING;
}
else if ((src->arg_def & (TRC_ARGS_CONN|TRC_ARGS_STRM)) && strcmp(name, "backend") == 0) {
HA_ATOMIC_STORE(&src->lockon, TRACE_LOCKON_BACKEND);
@@ -597,8 +618,10 @@
HA_ATOMIC_STORE(&src->lockon, TRACE_LOCKON_ARG4);
HA_ATOMIC_STORE(&src->lockon_ptr, NULL);
}
- else
- return cli_err(appctx, "Unsupported lock-on criterion");
+ else {
+ *msg = "Unsupported lock-on criterion";
+ return LOG_ERR;
+ }
}
else if (strcmp(args[2], "verbosity") == 0) {
const char *name = args[3];
@@ -618,7 +641,8 @@
nd->name, nd->desc);
}
trash.area[trash.data] = 0;
- return cli_msg(appctx, LOG_WARNING, trash.area);
+ *msg = trash.area;
+ return LOG_WARNING;
}
if (strcmp(name, "quiet") == 0)
@@ -626,22 +650,45 @@
else if (!src->decoding || !src->decoding[0].name) {
if (strcmp(name, "default") == 0)
HA_ATOMIC_STORE(&src->verbosity, 1);
- else
- return cli_err(appctx, "No such verbosity level");
+ else {
+ *msg = "No such verbosity level";
+ return LOG_ERR;
+ }
} else {
for (nd = src->decoding; nd->name && nd->desc; nd++)
if (strcmp(name, nd->name) == 0)
break;
- if (!nd->name || !nd->desc)
- return cli_err(appctx, "No such verbosity level");
+ if (!nd->name || !nd->desc) {
+ *msg = "No such verbosiry level";
+ return LOG_ERR;
+ }
HA_ATOMIC_STORE(&src->verbosity, (nd - src->decoding) + 1);
}
}
+ else {
+ *msg = "Unknown trace keyword";
+ return LOG_ERR;
+ }
- else
- return cli_err(appctx, "Unknown trace keyword");
+ return 0;
+
+}
+
+/* parse the command, returns 1 if a message is returned, otherwise zero */
+static int cli_parse_trace(char **args, char *payload, struct appctx *appctx, void *private)
+{
+ const char *msg;
+ int severity;
+
+ if (!cli_has_level(appctx, ACCESS_LVL_OPER))
+ return 1;
+
+ severity = trace_parse_statement(args, &msg);
+ if (msg)
+ return cli_msg(appctx, severity, msg);
+ /* total success */
return 0;
}