[DTrace-devel] [PATCH 3/4] Fix fbt:::return and pid:::return arg1

[Eugene Loh]

On 5/4/22 2:09 AM, Kris Van Hees wrote:

> On Wed, May 04, 2022 at 01:01:43AM -0400, eugene.loh--- via DTrace-devel wrote:
>> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
>>   /*
>> - * Copy arguments from a dt_pt_regs structure referenced by the 'rp' argument.
>> + * Copy arguments for entry probes.
>> + *
>> + * A dt_pt_regs structure is referenced by the 'rp' argument.
>>    *
>>    * The caller must ensure that %r7 contains the value set by the
>>    * dt_cg_tramp_prologue*() functions.
>>    */
>>   void
>> -dt_cg_tramp_copy_args_from_regs(dt_pcb_t *pcb, int rp)
>> +dt_cg_tramp_copy_entry_args(dt_pcb_t *pcb, int rp)
> Sorry, but no.  This function exists specifically to copy the values passed
> for a probe firing into the mstate.  It does not matter whether this is for
> entry probes or not.  In fact, various providers do not use entry and return
> probes.  Let's not sacrifice the common case in favour of a specific case.
I wanted to check in on this.  Yes we're copying args, but we get them 
where we must, whether from regs or the stack.  So to me the "from regs" 
seems misleading.  But I do not insist on fixing the name.  Just 
checking in.

Also, when we discussed dt_cg_subr_path_helper() in the past, you 
suggested naming the function narrowly, per existing usage.  If the 
usage would expand -- which it has, to dt_cg_subr_arg_to_tstring() -- we 
could rename it accordingly.  This time, we are only using the 
tramp-copy-args function for copying some entry-probe args.  So, the 
same rationale does not apply?  Name the function for its actual use:  
copying function args to become probe args for some entry probes?  If 
the usage changes in the future, rename then?