[DTrace-devel] [PATCH 01/04] cc: allow probe_info failures to report an error

[Eugene Loh]

The subject line was confusing to me since probe_info() already does 
report errors.  So I wondered what you really meant.  And why cc?  How 
about something like:

     Pass probe_info() error codes up the call stack

     The function probe_info() is provider-specific.  It
     returns an error code, but that code is ignored.

     Pass the error code up the callstack:
         (various call sites)
         -> dt_probe_info()
         -> dt_probe_args_info()
         -> prov->impl->probe_info()

Incidentally, dt_stmt_probe() and info_stmt() do not do anything with 
the return values.  Is that okay?

Also, the patch doesn't have any tests.  Maybe more stuff needs to be 
done before it can be tested.  So, I'll have more to say about that in a 
later patch.

Finally:

On 11/22/23 10:49, Kris Van Hees via DTrace-devel wrote:
> diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> -static int
> +int
>   dt_bpf_lockmem_error(dtrace_hdl_t *dtp, const char *msg)
>   {
>   	return dt_bpf_error(dtp, "%s:\n"

The lockmem stuff should be in the next patch?

> diff --git a/libdtrace/dt_bpf.h b/libdtrace/dt_bpf.h
> @@ -12,6 +12,7 @@
> +#include <dt_impl.h>
>   struct dtrace_hdl;

Is that include file really needed?  Specifically, we declare "struct 
dtrace_hdl" right there.  Then we have declarations like this:

     $ grep hdl libdtrace/dt_bpf.h
     struct dtrace_hdl;
     extern int dt_bpf_gmap_create(struct dtrace_hdl *);
     extern int dt_bpf_lockmem_error(dtrace_hdl_t *dtp, const char *msg);
     extern int dt_bpf_make_progs(struct dtrace_hdl *, uint_t);
     extern int dt_bpf_load_progs(struct dtrace_hdl *, uint_t);
     extern void dt_bpf_init_helpers(struct dtrace_hdl *dtp);

using (struct dtrace_hdl) while the definitions in dt_bpf.c are:

     dt_bpf_gmap_create(dtrace_hdl_t *dtp)
     dt_bpf_make_progs(dtrace_hdl_t *dtp, uint_t cflags)
     dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
     dt_bpf_init_helpers(dtrace_hdl_t *dtp)

in terms of dtrace_hdl_t.  So introducing dt_impl.h and handling 
lockmem_error() seems unnecessary, or at least inconsistent.

> @@ -59,6 +60,7 @@ extern int dt_bpf_map_lookup_inner(int fd, const void *okey, const void *ikey,
> +extern int dt_bpf_lockmem_error(dtrace_hdl_t *dtp, const char *msg);

The lockmem stuff should be in the next patch?