[DTrace-devel] [PATCH v2 1/3] Rename "path" helper to "tstring1" helper

[Kris Van Hees]

On Thu, Apr 21, 2022 at 04:57:47PM -0400, Eugene Loh via DTrace-devel wrote:
> R-b with the proposed changes?

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

for the modified version, and added to dev

> 
> On 4/19/22 11:32 PM, Kris Van Hees wrote:
> > On Fri, Mar 18, 2022 at 03:36:42PM -0400, eugene.loh--- via DTrace-devel wrote:
> > > From: Eugene Loh <eugene.loh at oracle.com>
> > > 
> > > When the code generator sets up a call to a precompiled BPF function,
> > > it emits a lot of boilerplate code.  The implementations of DTrace
> > > subroutines typically make such calls.  So there is potential for
> > > much code reuse.
> > > 
> > > The "path" helper is one such attempt.  Since its usefulness is
> > > broader than just for a "path name" argument, rename this helper to
> > > suggest its broader usage more clearly.
> > > 
> > > Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> > > ---
> > >   libdtrace/dt_cg.c | 39 +++++++++++++++++++++------------------
> > >   1 file changed, 21 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> > > index c40fef41..b8325e44 100644
> > > --- a/libdtrace/dt_cg.c
> > > +++ b/libdtrace/dt_cg.c
> > > @@ -3316,24 +3316,27 @@ dt_cg_array_op(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> > >   }
> > >   /*
> > > - * This function is a helper function for subroutines that take a path
> > > - * argument and return the tstring resulting from applying a given BPF
> > > - * function (passed by name) on it.
> > > + * This helper emits code to call a precompiled BPF function (named
> > > + * by fname) that is of return type void and takes two arguments:
> > > + *   - one input value
> > > + *   - a pointer to an output tstring, allocated here
> > >    */
> > >   static void
> > > -dt_cg_subr_path_helper(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
> > > -		       const char *fname)
> > > +dt_cg_subr_tstring1_helper(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
> > > +			   const char *fname)
> > s/dt_cg_subr_tstring1_helper/dt_cg_subr_arg_to_tstring/
> > 
> > >   {
> > >   	dt_ident_t	*idp;
> > > -	dt_node_t	*str = dnp->dn_args;
> > > +	dt_node_t	*arg = dnp->dn_args;
> > > -	TRACE_REGSET("    subr-path_helper:Begin");
> > > -	dt_cg_node(str, dlp, drp);
> > > -	dt_cg_check_notnull(dlp, drp, str->dn_reg);
> > > +	TRACE_REGSET("    subr-tstring1_helper:Begin");
> > s/subr-tstring1_helper/subr-arg-to-tstring/
> > 
> > > -	/*
> > > -	 * The result needs be be a temporary string, so we request one.
> > > -	 */
> > > +	/* handle the one "input value" */
> > > +	/* (its type matters only as to whether we check it is null */
> > > +	dt_cg_node(arg, dlp, drp);
> > > +	if (dt_node_is_pointer(arg) || dt_node_is_string(arg))
> > > +		dt_cg_check_notnull(dlp, drp, arg->dn_reg);
> > This should be changed to support alloca pointers:
> > 
> > 	if (dt_node_is_pointer(arg) || dt_node_is_string(arg))
> > 		dt_cg_check_ptr_arg(dlp, drp, arg);
> > 
> > > +
> > > +	/* allocate the temporary string */
> > >   	dnp->dn_reg = dt_regset_alloc(drp);
> > >   	if (dnp->dn_reg == -1)
> > >   		longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> > > @@ -3346,9 +3349,9 @@ dt_cg_subr_path_helper(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
> > >   	if (dt_regset_xalloc_args(drp) == -1)
> > >   		longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> > > -	emit(dlp, BPF_MOV_REG(BPF_REG_1, str->dn_reg));
> > > -	dt_regset_free(drp, str->dn_reg);
> > > -	dt_cg_tstring_free(yypcb, str);
> > > +	emit(dlp, BPF_MOV_REG(BPF_REG_1, arg->dn_reg));
> > > +	dt_regset_free(drp, arg->dn_reg);
> > > +	dt_cg_tstring_free(yypcb, arg);
> > How about:
> > 
> > 	if (dt_node_is_string(arg))
> > 		dt_cg_tstring_free(yypcb, arg);
> > 
> > >   	emit(dlp,  BPF_MOV_REG(BPF_REG_2, dnp->dn_reg));
> > > @@ -3359,19 +3362,19 @@ dt_cg_subr_path_helper(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
> > >   	dt_regset_free_args(drp);
> > >   	dt_regset_free(drp, BPF_REG_0);
> > > -	TRACE_REGSET("    subr-path_helper:End  ");
> > > +	TRACE_REGSET("    subr-tstring1_helper:End  ");
> > >   }
> > >   static void
> > >   dt_cg_subr_basename(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> > >   {
> > > -	dt_cg_subr_path_helper(dnp, dlp, drp, "dt_basename");
> > > +	dt_cg_subr_tstring1_helper(dnp, dlp, drp, "dt_basename");
> > >   }
> > >   static void
> > >   dt_cg_subr_dirname(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> > >   {
> > > -	dt_cg_subr_path_helper(dnp, dlp, drp, "dt_dirname");
> > > +	dt_cg_subr_tstring1_helper(dnp, dlp, drp, "dt_dirname");
> > >   }
> > >   /*
> > > -- 
> > > 2.18.4
> > > 
> > > 
> > > _______________________________________________
> > > DTrace-devel mailing list
> > > DTrace-devel at oss.oracle.com
> > > https://oss.oracle.com/mailman/listinfo/dtrace-devel
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel