Re: [PATCH 1/3] parser: add dt_node_is_tstring()

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

On Wed, Jul 16, 2025 at 12:43:10AM -0400, Eugene Loh wrote:
> Reviewed-by: Eugene Loh <eugene.loh@oracle.com>
> 
> Two minor comments:
> 
> 1.  It is practically unheard of in our code base to start the switch
> statement with the default case.  With maybe one exception, we put the
> default case at the end.

I know, but I wanted to really clearly communicate that the most common case
will be that the node does not have a tstring, i.e. default is the most common
case.

> 2.  How about using this function in dt_cg_tstring_free() so that it becomes
> 
>     static void
>     dt_cg_tstring_free(dt_pcb_t *pcb, dt_node_t *dnp)
>     {
>         if (dt_node_is_tstring(dnp) {
>             dt_cg_tstring_xfree(pcb, dnp->dn_tstring->dn_value);
>             dnp->dn_tstring = NULL;
>         }
>     }

Good idea.

> On 7/15/25 15:50, Kris Van Hees wrote:
> 
> > Returns 1 if the given node has a tstring associated with it.  Internal
> > change to make implementing optimized tstring-handling easier.
> > 
> > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> > ---
> >   libdtrace/dt_parser.c | 15 +++++++++++++++
> >   libdtrace/dt_parser.h |  1 +
> >   2 files changed, 16 insertions(+)
> > 
> > diff --git a/libdtrace/dt_parser.c b/libdtrace/dt_parser.c
> > index eefe8341..f777a910 100644
> > --- a/libdtrace/dt_parser.c
> > +++ b/libdtrace/dt_parser.c
> > @@ -1227,6 +1227,21 @@ dt_node_is_actfunc(const dt_node_t *dnp)
> >   	    dnp->dn_ident->di_kind == DT_IDENT_ACTFUNC;
> >   }
> > +int
> > +dt_node_is_tstring(const dt_node_t *dnp)
> > +{
> > +	switch (dnp->dn_kind) {
> > +	default:
> > +		return 0;
> > +	case DT_NODE_FUNC:
> > +	case DT_NODE_OP1:
> > +	case DT_NODE_OP2:
> > +	case DT_NODE_OP3:
> > +	case DT_NODE_DEXPR:
> > +		return dnp->dn_tstring != NULL;
> > +	}
> > +}
> > +
> >   /*
> >    * The original rules for integer constant typing are described in K&R[A2.5.1].
> >    * However, since we support long long, we instead use the rules from ISO C99
> > diff --git a/libdtrace/dt_parser.h b/libdtrace/dt_parser.h
> > index 13f3cc99..ff32a84a 100644
> > --- a/libdtrace/dt_parser.h
> > +++ b/libdtrace/dt_parser.h
> > @@ -182,6 +182,7 @@ extern int dt_node_is_ptrcompat(const dt_node_t *, const dt_node_t *,
> >   extern int dt_node_is_argcompat(const dt_node_t *, const dt_node_t *);
> >   extern int dt_node_is_posconst(const dt_node_t *);
> >   extern int dt_node_is_actfunc(const dt_node_t *);
> > +extern int dt_node_is_tstring(const dt_node_t *);
> >   extern dt_node_t *dt_node_int(uintmax_t);
> >   extern dt_node_t *dt_node_string(char *);