Re: [PATCH 1/4] Rename _DTRACE_VERSION

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

On Sat, Feb 08, 2025 at 02:06:19PM -0500, eugene.loh@oracle.com wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> There are many DTrace version numbers (for version, API version,
> package version, etc.).  Meanwhile, _DTRACE_VERSION is not a
> version number at all.  It's a preprocessor macro in USDT .h header
> files.  Prior to commit e2fb0ecd9
> ("Ensure multiple passes through dtrace -G work."), it was perhaps
> not even set.  With that commit, it was always set to 1, with
> the rationale:
> 
>     Also add an explicit define for _DTRACE__VERSION in the generated
>     header file from 'dtrace -h' invocations.  This seems silly, but
>     it is there to give people a skeleton to work with if they want to
>     pre-generate header files and select whether to actually compile
>     on the probes at a later time.
> 
> Rename to _DTRACE_HEADER for better clarity.  Define it only once
> per file.

Based on the rationale, I would think that something like _DTRACE_USE_USDT
or somethng similar would be better?  Since the purpose seems to be to allow
(after the header file is generated) for someone to change that 1 to 0 to
disable the USDT probes to be compiled in.

In fact, if we want to support this in a more developer-friendly way, we
might as well change the generted line to be more like:

#ifndef _DTRACE_USE_USDT
# define _DTARCE_USE_USDT 1
#endif

so that a package could e.g. allow a configure option or something to set it
to 1 or 0, dypassing the need to manually change the file.

> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> ---
>  libdtrace/dt_program.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/libdtrace/dt_program.c b/libdtrace/dt_program.c
> index 23b91fb2e..c6fdafb47 100644
> --- a/libdtrace/dt_program.c
> +++ b/libdtrace/dt_program.c
> @@ -505,13 +505,12 @@ dt_header_provider(dtrace_hdl_t *dtp, dt_provider_t *pvp, FILE *out)
>  	info.dthi_pfname = alloca(strlen(pvp->desc.dtvd_name) + 1 + i);
>  	dt_header_fmt_func(info.dthi_pfname, pvp->desc.dtvd_name);
>  
> -	if (fprintf(out, "#define _DTRACE_VERSION 1\n\n"
> -			 "#if _DTRACE_VERSION\n\n") < 0)
> +	if (fprintf(out, "#if _DTRACE_HEADER\n\n") < 0)
>  		return dt_set_errno(dtp, errno);
>  
>  	if (dt_idhash_iter(pvp->pv_probes, dt_header_probe, &info) != 0)
>  		return -1; /* dt_errno is set for us */
> -	if (fprintf(out, "\n\n") < 0)
> +	if (fprintf(out, "\n") < 0)
>  		return dt_set_errno(dtp, errno);
>  	if (dt_idhash_iter(pvp->pv_probes, dt_header_decl, &info) != 0)
>  		return -1; /* dt_errno is set for us */
> @@ -560,6 +559,9 @@ dtrace_program_header(dtrace_hdl_t *dtp, FILE *out, const char *fname)
>  		"#endif\n\n") < 0)
>  		return -1;
>  
> +	if (fprintf(out, "#define _DTRACE_HEADER 1\n\n") < 0)
> +		return -1;
> +
>  	while ((pvp = dt_htab_next(dtp->dt_provs, &it)) != NULL) {
>  		if (dt_header_provider(dtp, pvp, out) != 0) {
>  			dt_htab_next_destroy(it);
> -- 
> 2.43.5
>