[DTrace-devel] [PATCH v2 01/04] probe: propagate dt_probe_args_info() failures to dt_probe_info()
Eugene Loh
eugene.loh at oracle.com
Thu Nov 30 23:38:46 UTC 2023
Another one not in my inbox. Looks like Nick got to it. Just one
minute thing. In dt_probe_args_info(), you have
+ if (rc == -1)
+ return rc;
How about making that "return -1" to make it infinitesimally easier to
figure out what the possible return values are.
On 11/30/23 12:11, Nick Alcock via DTrace-devel wrote:
> On 30 Nov 2023, Kris Van Hees via DTrace-devel stated:
>
>> Failures in dt_probe_args_info() were ignored by dt_probe_info() and
>> that could lead to strange behaviour. E.g. if lockmem issues cause
>> the logic in the raw tracepoint argument count determination to fail,
>> the probe was reported as not having any arguments. This patch will
>> make it possible to add code to accurately report such failures
>> rather than silently ignoring them.
> Good idea, but...
>
>> diff --git a/cmd/dtrace.c b/cmd/dtrace.c
>> index e7ca9e4c..9c820686 100644
>> --- a/cmd/dtrace.c
>> +++ b/cmd/dtrace.c
>> @@ -323,6 +323,8 @@ info_stmt(dtrace_hdl_t *dtp, dtrace_prog_t *pgp,
>>
>> if (dtrace_probe_info(dtp, pdp, &p) == 0)
>> print_probe_info(&p);
>> + else
>> + return -1;
> So dtrace_probe_info()'s error returns do not emit any errors anywhere:
> they just set the dtrace_errno. On error, this doesn't emit any errors either...
>
>> *last = edp;
>> return 0;
>> @@ -417,8 +419,12 @@ list_probe(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp, void *arg)
>> oprintf("%5d %10s %17s %33s %s\n",
>> pdp->id, pdp->prv, pdp->mod, pdp->fun, pdp->prb);
>>
>> - if (g_verbose && dtrace_probe_info(dtp, pdp, &p) == 0)
>> - print_probe_info(&p);
>> + if (g_verbose) {
>> + if (dtrace_probe_info(dtp, pdp, &p) == 0)
>> + print_probe_info(&p);
>> + else
>> + return -1;
>> + }
> ... so nor does this; and then this call, which you didn't touch:
>
> case DMODE_LIST:
> [...] if (g_cmdc == 0)
> dtrace_probe_iter(g_dtp, NULL, list_probe, NULL);
>
> dtrace_close(g_dtp);
> return g_status;
>
> ... exits early without printing the dtrace_errmsg. You might want to
> check all the dtrace_probe_iter()s that call list_probe() for error
> returns, perhaps?
>
>> @@ -1426,8 +1432,10 @@ main(int argc, char *argv[])
>> for (i = 0; i < g_cmdc; i++)
>> list_prog(&g_cmdv[i]);
>>
>> - if (g_cmdc == 0)
>> - dtrace_probe_iter(g_dtp, NULL, list_probe, NULL);
>> + if (g_cmdc == 0) {
>> + if (dtrace_probe_iter(g_dtp, NULL, list_probe, NULL) < 0)
>> + dfatal(NULL); /* dtrace_errmsg() only */
>> + }
> Like you do here :)
>
> The changes to dt_probe.c look good.
>
More information about the DTrace-devel
mailing list