[DTrace-devel] [PATCH 5/7] options: centralized error handling
Nick Alcock
nick.alcock at oracle.com
Tue May 2 17:12:20 UTC 2023
The commit preceding this one improved everything other than erroneous
options. Errors were simply thrown away, because dtrace_setopt() no
longer emits errors when dtp is NULL.
So add a setopt error handler which is called on dtrace_setopt() error
(including on delayed dtrace_setopt() calls inside dtrace_open()) and
can be used to emit suitable messages. It can't quite do the same
thing, because it's hard to tell when -x was used, but it can get very
close most of the time.
For now, we use worse error messages than we might, to prove that
errors are unchanged: the next commit improves them.
Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
---
cmd/dtrace.c | 129 ++++++++++++++++++++++++++++++----------
libdtrace/dt_options.c | 55 +++++++++++++----
libdtrace/dtrace.h | 21 +++++++
libdtrace/libdtrace.ver | 1 +
4 files changed, 164 insertions(+), 42 deletions(-)
diff --git a/cmd/dtrace.c b/cmd/dtrace.c
index 8af798774953e..0755a39e3b8b1 100644
--- a/cmd/dtrace.c
+++ b/cmd/dtrace.c
@@ -546,6 +546,71 @@ setopthandler(const dtrace_setoptdata_t *data, void *arg)
return DTRACE_HANDLE_OK;
}
+/*ARGSUSED*/
+static void
+setopt_err_handler(const dtrace_setopterr_t *data, void *unused)
+{
+ /*
+ * This list should be kept in sync with the list of command-line
+ * arguments in pass 2 in main(). (But if it's not, the only effect is
+ * slightly worse error output.)
+ */
+ struct option_to_arg {
+ const char *option;
+ const char *arg;
+ } args[] = {
+ { "bufsize", "-b" },
+ { "define", "-D" },
+ { "flowindent", "-F" },
+ { "cpphdrs", "-H" },
+ { "incdir", "-I" },
+ { "libdir", "-L" },
+ { "quiet", "-q" },
+ { "undef", "-U" },
+ { "destructive", "-w" },
+ { "stdc", "-X" },
+ { NULL }};
+ struct option_to_arg *argp;
+ const char *arg = NULL;
+ const char *opttype;
+
+ /*
+ * This is usually called by dtrace_open, which has initialized the dtp
+ * but not yet returned, so the caller has not had a chance to assign it
+ * to g_dtp (used by dfatal). It passes down its dtp for this purpose:
+ * do so here.
+ */
+ if (g_dtp == NULL)
+ g_dtp = data->dtso_handle;
+
+ for (argp = args; argp->arg != NULL; argp++) {
+ if (strcmp(argp->arg, data->dtso_opt) == 0) {
+ arg = argp->arg;
+ break;
+ }
+ }
+
+ if (arg) {
+ if (data->dtso_val != NULL)
+ dfatal("failed to set %s=%s", arg, data->dtso_val);
+ else
+ dfatal("failed to set %s", arg);
+ }
+
+ /*
+ * Handle -x, which can control many options. We assume that anything
+ * so far unhandled is -x, except iff setoptenv is in use. This may
+ * cause the occasional bit of confusing output, but it should be right
+ * most of the time.
+ */
+ if (!data->dtso_setoptenv)
+ opttype = "-x";
+ else
+ opttype = "DTrace option";
+
+ dfatal("failed to set %s %s", opttype, data->dtso_opt);
+}
+
#define BUFDUMPHDR(hdr) \
printf("%s: %s%s\n", g_pname, hdr, strlen(hdr) > 0 ? ":" : "");
@@ -1080,9 +1145,15 @@ main(int argc, char *argv[])
}
}
+ /*
+ * Set the option handler to one that complains systematically about
+ * bad command-line options.
+ */
+ dtrace_handle_setopt_err(setopt_err_handler, NULL);
+
/*
* Set default options. Because g_dtp is NULL these go into a cache of
- * default options to be applied at dtrace_open time.
+ * default options to be validated and applied at dtrace_open time.
*/
dtrace_setopt(g_dtp, "bufsize", "4m");
dtrace_setopt(g_dtp, "aggsize", "4m");
@@ -1125,9 +1196,7 @@ main(int argc, char *argv[])
while ((c = getopt(argc, argv, DTRACE_OPTSTR)) != EOF) {
switch (c) {
case 'b':
- if (dtrace_setopt(g_dtp,
- "bufsize", optarg) != 0)
- dfatal("failed to set -b %s", optarg);
+ dtrace_setopt(g_dtp, "bufsize", optarg);
break;
case 'B':
@@ -1139,8 +1208,7 @@ main(int argc, char *argv[])
break;
case 'D':
- if (dtrace_setopt(g_dtp, "define", optarg) != 0)
- dfatal("failed to set -D %s", optarg);
+ dtrace_setopt(g_dtp, "define", optarg);
break;
case 'f':
@@ -1151,13 +1219,11 @@ main(int argc, char *argv[])
break;
case 'F':
- if (dtrace_setopt(g_dtp, "flowindent", 0) != 0)
- dfatal("failed to set -F");
+ dtrace_setopt(g_dtp, "flowindent", 0);
break;
case 'H':
- if (dtrace_setopt(g_dtp, "cpphdrs", 0) != 0)
- dfatal("failed to set -H");
+ dtrace_setopt(g_dtp, "cpphdrs", 0);
break;
case 'i':
@@ -1168,13 +1234,11 @@ main(int argc, char *argv[])
break;
case 'I':
- if (dtrace_setopt(g_dtp, "incdir", optarg) != 0)
- dfatal("failed to set -I %s", optarg);
+ dtrace_setopt(g_dtp, "incdir", optarg);
break;
case 'L':
- if (dtrace_setopt(g_dtp, "libdir", optarg) != 0)
- dfatal("failed to set -L %s", optarg);
+ dtrace_setopt(g_dtp, "libdir", optarg);
break;
case 'm':
@@ -1199,8 +1263,7 @@ main(int argc, char *argv[])
break;
case 'q':
- if (dtrace_setopt(g_dtp, "quiet", 0) != 0)
- dfatal("failed to set -q");
+ dtrace_setopt(g_dtp, "quiet", 0);
break;
case 'o':
@@ -1219,26 +1282,22 @@ main(int argc, char *argv[])
break;
case 'U':
- if (dtrace_setopt(g_dtp, "undef", optarg) != 0)
- dfatal("failed to set -U %s", optarg);
+ dtrace_setopt(g_dtp, "undef", optarg);
break;
case 'w':
- if (dtrace_setopt(g_dtp, "destructive", 0) != 0)
- dfatal("failed to set -w");
+ dtrace_setopt(g_dtp, "destructive", 0);
break;
case 'x':
if ((p = strchr(optarg, '=')) != NULL)
*p++ = '\0';
- if (dtrace_setopt(g_dtp, optarg, p) != 0)
- dfatal("failed to set -x %s", optarg);
+ dtrace_setopt(g_dtp, optarg, p);
break;
case 'X':
- if (dtrace_setopt(g_dtp, "stdc", optarg) != 0)
- dfatal("failed to set -X %s", optarg);
+ dtrace_setopt(g_dtp, "stdc", optarg);
break;
case 'Z':
@@ -1263,6 +1322,20 @@ main(int argc, char *argv[])
return E_USAGE;
}
+ /*
+ * Open libdtrace.
+ */
+ g_dtp = dtrace_open(DTRACE_VERSION, g_oflags, &err);
+ if (g_dtp == NULL)
+ fatal("failed to initialize dtrace: %s\n",
+ dtrace_errmsg(NULL, err));
+
+ /*
+ * We don't need a setopt err handler any more: dtrace is initialized
+ * and dtrace_setopt will return immediately on errors.
+ */
+ dtrace_handle_setopt_err(NULL, NULL);
+
/*
* Turn on testing mode if requested. This only affects dtrace.c, so is
* not controlled by a dtrace option. This quiesces a variety of
@@ -1275,14 +1348,6 @@ main(int argc, char *argv[])
"testing", g_pname);
}
- /*
- * Open libdtrace.
- */
- g_dtp = dtrace_open(DTRACE_VERSION, g_oflags, &err);
- if (g_dtp == NULL)
- fatal("failed to initialize dtrace: %s\n",
- dtrace_errmsg(NULL, err));
-
/*
* In our third pass we handle any command-line options related to
* grabbing or creating victim processes. The behavior of these calls
diff --git a/libdtrace/dt_options.c b/libdtrace/dt_options.c
index 2dd1a3d18b597..364acf60047b3 100644
--- a/libdtrace/dt_options.c
+++ b/libdtrace/dt_options.c
@@ -24,6 +24,12 @@
#include <dt_string.h>
#include <libproc.h>
+/*
+ * Global option-checking handler.
+ */
+static dtrace_handle_setopt_err_f *handle_setopt_err;
+static void *handle_setopt_arg;
+
static int
dt_opt_agg(dtrace_hdl_t *dtp, const char *arg, uintptr_t option)
{
@@ -1068,8 +1074,10 @@ typedef struct dt_option {
*/
static const dt_option_t _dtrace_btoptions[] = {
{ "ctfpath", dt_opt_ctfa_path },
+ { "debug", dt_opt_debug },
{ "modpath", dt_opt_module_path },
- { "procfspath", dt_opt_procfs_path }
+ { "procfspath", dt_opt_procfs_path },
+ { NULL }
};
/*
@@ -1086,7 +1094,6 @@ static const dt_option_t _dtrace_ctoptions[] = {
{ "cpppath", dt_opt_cpp_path },
{ "ctypes", dt_opt_ctypes },
{ "defaultargs", dt_opt_cflags, DTRACE_C_DEFARG },
- { "debug", dt_opt_debug },
{ "debugassert", dt_opt_debug_assert },
{ "define", dt_opt_cpp_opts, (uintptr_t)"-D" },
{ "disasm", dt_opt_disasm },
@@ -1296,10 +1303,23 @@ dtrace_setopt(dtrace_hdl_t *dtp, const char *opt, const char *val)
* tracing is active.
*/
if (strcmp(op->o_name, opt) == 0) {
+ int err;
+
if (*pops == _dtrace_rtoptions && dtp->dt_active)
- return dt_set_errno(dtp, EDT_ACTIVE);
+ err = dt_set_errno(dtp, EDT_ACTIVE);
else
- return op->o_func(dtp, val, op->o_option);
+ err = op->o_func(dtp, val, op->o_option);
+
+ if (err != 0 && handle_setopt_err != NULL) {
+ dtrace_setopterr_t opterr;
+
+ opterr.dtso_handle = dtp;
+ opterr.dtso_opt = opt;
+ opterr.dtso_val = val;
+ opterr.dtso_setoptenv = 0;
+ handle_setopt_err(&opterr, handle_setopt_arg);
+ }
+ return err;
}
}
}
@@ -1351,6 +1371,7 @@ dtrace_setoptenv(dtrace_hdl_t *dtp, const char *prefix)
for (op = *pops; op->o_name != NULL; op++) {
if ((val = dt_opt_getenv_prefix(dtp, op->o_name,
prefix)) != NULL) {
+ int err;
if (dtp == NULL) {
set_default_option(op->o_name, val);
@@ -1359,17 +1380,31 @@ dtrace_setoptenv(dtrace_hdl_t *dtp, const char *prefix)
/*
* Only dynamic run-time options may be
- * set while tracing is active. Since
- * env var application is a wholesale
- * thing, just skip things we can't
- * apply.
+ * set while tracing is active.
*/
if (*pops == _dtrace_rtoptions &&
dtp->dt_active)
- continue;
+ err = dt_set_errno(dtp, EDT_ACTIVE);
+ else
+ err = op->o_func(dtp, val, op->o_option);
- op->o_func(dtp, val, op->o_option);
+ if (err != 0 && handle_setopt_err != NULL) {
+ dtrace_setopterr_t opterr;
+
+ opterr.dtso_handle = dtp;
+ opterr.dtso_opt = op->o_name;
+ opterr.dtso_val = val;
+ opterr.dtso_setoptenv = 1;
+ handle_setopt_err(&opterr, handle_setopt_arg);
+ }
}
}
}
}
+
+void
+dtrace_handle_setopt_err(dtrace_handle_setopt_err_f *hdlr, void *arg)
+{
+ handle_setopt_err = hdlr;
+ handle_setopt_arg = arg;
+}
diff --git a/libdtrace/dtrace.h b/libdtrace/dtrace.h
index 18080c5db5203..edb6abf50c310 100644
--- a/libdtrace/dtrace.h
+++ b/libdtrace/dtrace.h
@@ -350,6 +350,27 @@ typedef int dtrace_handle_setopt_f(const dtrace_setoptdata_t *data, void *arg);
extern int dtrace_handle_setopt(dtrace_hdl_t *dtp,
dtrace_handle_setopt_f *hdlr, void *arg);
+/*
+ * Called for any error in dtrace_setopt(), at the time of option application
+ * (which may be some time after the dtrace_setopt() call, at any later time
+ * when dtrace_open is called: make sure the arg lives long enough). The
+ * handler is *global*, not tied to a dtrace handle: set it to NULL to
+ * turn it off again.
+ *
+ * If dtso_setoptenv is set, the handler was called as a result of a
+ * dtrace_setoptenv call.
+ *
+ * The handler may be called repeatedly if many options have errors.
+ */
+typedef struct dtrace_setopterr {
+ dtrace_hdl_t *dtso_handle;
+ const char *dtso_opt;
+ const char *dtso_val;
+ int dtso_setoptenv;
+} dtrace_setopterr_t;
+typedef void dtrace_handle_setopt_err_f(const dtrace_setopterr_t *data, void *arg);
+extern void dtrace_handle_setopt_err(dtrace_handle_setopt_err_f *hdlr, void *arg);
+
/*
* DTrace Aggregate Interface
*/
diff --git a/libdtrace/libdtrace.ver b/libdtrace/libdtrace.ver
index 3886c18e4abd8..f293407c7a426 100644
--- a/libdtrace/libdtrace.ver
+++ b/libdtrace/libdtrace.ver
@@ -44,6 +44,7 @@ LIBDTRACE_1.0 {
dtrace_handle_err;
dtrace_handle_proc;
dtrace_handle_setopt;
+ dtrace_handle_setopt_err;
dtrace_id2desc;
dtrace_init;
dtrace_lookup_by_addr;
--
2.39.1.268.g9de2f9a303
More information about the DTrace-devel
mailing list