Re: [PATCH 2/3] Defer stripping out "__data_loc" until we know that is useful

[Nick Alcock <nick.alcock@oracle.com>]

On 13 Sep 2025, eugene loh uttered the following:

> From: Eugene Loh <eugene.loh@oracle.com>
>
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

Reviewed-by: Nick Alcock <nick.alcock@oracle.com>

with the tiny caveat below.

> ---
>  libdtrace/dt_provider_tp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/libdtrace/dt_provider_tp.c b/libdtrace/dt_provider_tp.c
> index 33821f2c7..a6f978e3f 100644
> --- a/libdtrace/dt_provider_tp.c
> +++ b/libdtrace/dt_provider_tp.c
> @@ -208,12 +208,13 @@ dt_tp_event_info(dtrace_hdl_t *dtp, FILE *f, int skip, tp_probe_t *tpp,
>  		p = buf;
>  		if (sscanf(buf, " field:%[^;]", p) <= 0)
>  			continue;
> -		sscanf(p, "__data_loc %[^;]", p);
>  
>  		/* We found a field: description - see if we should skip it. */
>  		if (argc < 0)
>  			goto skip;
>  
> +		sscanf(p, "__data_loc %[^;]", p);
> +

... why not defer the sscanf above it too? The manipulation of argc at
this point looks like this:

        argc = -skip;

(in getline loop)

                /* We found a field: description - see if we should skip it. */
                if (argc < 0)
                        goto skip;
(at loop end)
	skip:
                argc++;
        }


So if argc is < 0, it's going to *stay* like that until a given number
of getlines have happened. Doing more work than just skipping and
getlining is pointless. (This affects the next patch as much as this
one. More generally, it might be clearer to move skipping to a separate
getline loop entirely, above this one that also does all the other work.)

-- 
NULL && (void)