[DTrace-devel] [PATCH 07/17] providers: allow providers with no trampoline

[Nick Alcock]

On 1 Sep 2022, Eugene Loh via DTrace-devel uttered the following:

> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
>
> It's a simple patch and looks right, but I'm a little confused. When
> is the dt_cg.c diff needed?  I tried some pid$foo probes (with the
> later patch that removes .trampoline for that provider), and it seems
> that I never get trampoline==NULL even though I do see
> trampoline==NULL for the dt_bpf.c code site.  Is the dt_cg.c diff
> "just to be safe?"  "For some future case?"  Unnecessary?  Or am I
> missing something?

Just to be safe -- we guard against null trampolines in multiple places,
and the failure in dt_cg.c cannot happen because of the guard in
dt_bpf.c. But nonetheless, if trampolines are nullable, it makes sense
to me to not assume they are non-null just because we happen to be only
called from one caller which itself checks for this. That sort of
non-local, cross-file coupling, particularly implied coupling indicated
by a *lack* of code (or comments) at one site, makes me feel a bit ill :)
I mean yes it happens all the time, but I try not to encourage it when I
see it.

> On 8/10/22 18:06, Nick Alcock via DTrace-devel wrote:
>
>> These can be enabled just like any other, but generate no code.
>
> This sentence confuses me.  "These what?"  At the very least, "these providers"?  And presumably there are no such providers at the
> time of this patch.  How about dumping this sentence?

The first line of the commit log is "allow providers with no
trampoline", so, uh, yeah, normal English deixis would suggest that? :)

Rephrased anyway.

>> Meant for use by things like the pid provider which have two
>> probes associated with each system-side probe, one of which
>> gets fired and generates code for the others in its trampoline
>> method.
>
> This also confuses me.  How about, "In a future patch, the
> PID-specific provider dt_pid_proc will no longer have a trampoline." 

Ah, good point -- right now, as of this commit, both have trampolines.
Adjusted the phrasing to:

    providers: allow providers with no trampoline
    
    Such providers can be enabled just like any other, but generate no code.
    
    Meant for use by things like the pid provider which have two probes
    associated with each system-side probe, one of which gets fired and
    generates code for the others in its trampoline method. Right now, both
    probes get the same trampoline, but only one probe's trampoline is
    necessary. Doing this lets us drop the unnecessary ones in a future
    commit.

-- 
NULL && (void)