Re: [DTrace-devel] [PATCH 2/2] Use a consistent type for dtrace_consume()

[Kris Van Hees <kris.van.hees@oracle.com>]

On Tue, Aug 12, 2025 at 06:46:06PM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> For a long time, the type of dtrace_consume() has been
> - int                 according to libdtrace/dtrace.h
> - dtrace_workstatus_t according to libdtrace/dt_consume.c
> 
> With some recent compilers, however, this triggers the warning (redacted
> here):
> 
>     libdtrace/dt_consume.c:3041:1: warning: conflicting types
>     for ???dtrace_consume??? due to enum/integer mismatch;
>     have ???dtrace_workstatus_t(...)??? [-Wenum-int-mismatch]
>      3041 | dtrace_consume(...)
>           | ^~~~~~~~~~~~~~
>     In file included from libdtrace/dt_impl.h:14,
>                      from libdtrace/dt_consume.c:16:
>     libdtrace/dtrace.h:210:12: note: previous declaration of
>     ???dtrace_consume??? with type ???int(...)???
>       210 | extern int dtrace_consume(...)
>           |            ^~~~~~~~~~~~~~
> 
> which is a nuisance.
> 
> Note that dtrace_consume() is called from only one site, where its
> value is compared to DTRACE_WORKSTATUS_ERROR, which is an argument
> for the dtrace_workstatus_t type.
> 
> On the other hand, dtrace_consume() is defined to return a variety
> of values, like 0, dt_set_errno(), and dt_consume_cpu(), all of which
> are int, but also DTRACE_WORKSTATUS_OKAY, DTRACE_WORKSTATUS_ERROR,
> and rval, all of which are dtrace_workstatus_t.  But then rval itself
> is set to dtrace_workstatus_t dt_consume_begin() or int
> dt_consume_cpu().  So, there is simply no consistency here.
> 
> Having the prototype be dtrace_workstatus_t requires some amount
> of code refactoring.
> 
> Just change the definition to int and clean up the warning.
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

... I admit this was my fault.  I tried to refactor the code so that we could
    have dtrace_consume() return dtrace_workstatus_t, but only got this far.
    I agree that for now, changing the return type of dtrace_consume() is the
    best action to get rid of the compiler warning.  Maybe we can revisit the
    actual refactoring work later to clean this up properly.

> ---
>  libdtrace/dt_consume.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
> index 07b19d498..d9be563d9 100644
> --- a/libdtrace/dt_consume.c
> +++ b/libdtrace/dt_consume.c
> @@ -3037,7 +3037,7 @@ dt_consume_fini(dtrace_hdl_t *dtp)
>  	dt_htab_destroy(dtp->dt_spec_bufs);
>  }
>  
> -dtrace_workstatus_t
> +int
>  dtrace_consume(dtrace_hdl_t *dtp, FILE *fp, dtrace_consume_probe_f *pf,
>  	       dtrace_consume_rec_f *rf, void *arg)
>  {
> -- 
> 2.47.3
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel