public inbox for dtrace@lists.linux.dev
 help / color / mirror / Atom feed
* [PATCH 1/3] parser: add dt_node_is_tstring()
@ 2025-07-15 19:50 Kris Van Hees
  2025-07-16  4:43 ` Eugene Loh
  0 siblings, 1 reply; 3+ messages in thread
From: Kris Van Hees @ 2025-07-15 19:50 UTC (permalink / raw)
  To: dtrace, dtrace-devel

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 *);
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/3] parser: add dt_node_is_tstring()
  2025-07-15 19:50 [PATCH 1/3] parser: add dt_node_is_tstring() Kris Van Hees
@ 2025-07-16  4:43 ` Eugene Loh
  2025-07-16  5:16   ` Kris Van Hees
  0 siblings, 1 reply; 3+ messages in thread
From: Eugene Loh @ 2025-07-16  4:43 UTC (permalink / raw)
  To: Kris Van Hees, dtrace, dtrace-devel

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.

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;
         }
     }

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 *);

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/3] parser: add dt_node_is_tstring()
  2025-07-16  4:43 ` Eugene Loh
@ 2025-07-16  5:16   ` Kris Van Hees
  0 siblings, 0 replies; 3+ messages in thread
From: Kris Van Hees @ 2025-07-16  5:16 UTC (permalink / raw)
  To: Eugene Loh; +Cc: Kris Van Hees, dtrace, dtrace-devel

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 *);

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-07-16  5:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-15 19:50 [PATCH 1/3] parser: add dt_node_is_tstring() Kris Van Hees
2025-07-16  4:43 ` Eugene Loh
2025-07-16  5:16   ` Kris Van Hees

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox