Re: [PATCH] printf: fake type node is only needed for aggregation printing

Reviewed-by: Eugene Loh <[email protected]>

On 8/29/25 14:46, Kris Van Hees wrote:
> Regular printf validation was passing DTRACEACT_AGGREGATION as kind
> argument, even though the action was not doing anything with
> aggregations.  Pass DTRACEACT_NONE instead because the argument is not
> used in that case.
>
> Also, do not populate the fake type node if the validation is called
> for anything other than aggregations.
>
> Signed-off-by: Kris Van Hees <[email protected]>
> ---
>   libdtrace/dt_cg.c     |  2 +-
>   libdtrace/dt_printf.c | 34 +++++++++++++++++++---------------
>   2 files changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 7af3dd44b..77699c786 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -2482,7 +2482,7 @@ dt_cg_act_printf(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
>   	 */
>   	pfp = dt_printf_create(pcb->pcb_hdl, str);
>   	dt_printf_validate(pfp, DT_PRINTF_EXACTLEN, dnp->dn_ident, 1,
> -			   DTRACEACT_AGGREGATION, arg1);
> +			   DTRACEACT_NONE, arg1);
>   
>   	/*
>   	 * If no arguments are provided we will be printing a string constant.
> diff --git a/libdtrace/dt_printf.c b/libdtrace/dt_printf.c
> index 21ec74111..1e01d4687 100644
> --- a/libdtrace/dt_printf.c
> +++ b/libdtrace/dt_printf.c
> @@ -961,8 +961,8 @@ dt_printf_destroy(dt_pfargv_t *pfv)
>   }
>   
>   void
> -dt_printf_validate(dt_pfargv_t *pfv, uint_t flags,
> -    dt_ident_t *idp, int foff, dtrace_actkind_t kind, dt_node_t *dnp)
> +dt_printf_validate(dt_pfargv_t *pfv, uint_t flags, dt_ident_t *idp, int foff,
> +		   dtrace_actkind_t kind, dt_node_t *dnp)
>   {
>   	dt_pfargd_t *pfd = pfv->pfv_argv;
>   	const char *func = idp->di_name;
> @@ -980,21 +980,25 @@ dt_printf_validate(dt_pfargv_t *pfv, uint_t flags,
>   
>   	pfv->pfv_flags = flags;
>   
> -	/*
> -	 * We fake up a parse node representing the type that can be used with
> -	 * an aggregation result conversion, which -- for all but count() --
> -	 * is a signed quantity.
> -	 */
> -	if (kind != DT_AGG_COUNT)
> -		aggtype = "int64_t";
> -	else
> -		aggtype = "uint64_t";
> +	memset(&aggnode, 0, sizeof(aggnode));
> +
> +	if (flags & DT_PRINTF_AGGREGATION) {
> +		/*
> +		 * We fake up a parse node representing the type that can be
> +		 * used with an aggregation result conversion, which -- for all
> +		 * but count() -- is a signed quantity.
> +		 */
> +		if (kind != DT_AGG_COUNT)
> +			aggtype = "int64_t";
> +		else
> +			aggtype = "uint64_t";
>   
> -	if (dt_type_lookup(aggtype, &dtt) != 0)
> -		xyerror(D_TYPE_ERR, "failed to lookup agg type %s\n", aggtype);
> +		if (dt_type_lookup(aggtype, &dtt) != 0)
> +			xyerror(D_TYPE_ERR, "failed to lookup agg type %s\n",
> +				aggtype);
>   
> -	memset(&aggnode, 0, sizeof(aggnode));
> -	dt_node_type_assign(&aggnode, dtt.dtt_ctfp, dtt.dtt_type);
> +		dt_node_type_assign(&aggnode, dtt.dtt_ctfp, dtt.dtt_type);
> +	}
>   
>   	for (i = 0, j = 0; i < pfv->pfv_argc; i++, pfd = pfd->pfd_next) {
>   		const dt_pfconv_t *pfc = pfd->pfd_conv;