[DTrace-devel] [PATCH 6/9] Move some ustack code closer to stack code to prepare for consolidation

[Kris Van Hees]

I do not believe this patch is necessary.  For one, this could have been
done in the patch that introduces this function without any loss of clarity.
But also, as I mention in my comments on patch [7/9] om the series, I do not
believe that you need to split out the argument handling into separate
functions.

On Thu, Oct 05, 2023 at 05:14:04PM -0400, eugene.loh at oracle.com wrote:
> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  libdtrace/dt_cg.c | 72 +++++++++++++++++++++++------------------------
>  1 file changed, 36 insertions(+), 36 deletions(-)
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 389e4dba..6cae8814 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -2189,6 +2189,42 @@ dt_cg_stack_nframes(dtrace_hdl_t *dtp, dt_node_t *dnp)
>  	return nframes;
>  }
>  
> +static uint64_t
> +dt_cg_ustack_arg(dtrace_hdl_t *dtp, dt_node_t *dnp)
> +{
> +	int		nframes = dtp->dt_options[DTRACEOPT_USTACKFRAMES];
> +	int		strsize = 0;
> +	dt_node_t	*arg0 = dnp->dn_args;
> +	dt_node_t	*arg1 = arg0 != NULL ? arg0->dn_list : NULL;
> +
> +	if (nframes == DTRACEOPT_UNSET)
> +		nframes = _dtrace_ustackframes;
> +
> +	if (arg0 != NULL) {
> +		if (!dt_node_is_posconst(arg0))
> +			dnerror(arg0, D_USTACK_FRAMES, "ustack( ) argument #1 "
> +				"must be a non-zero positive integer "
> +				"constant\n");
> +
> +		nframes = arg0->dn_value;
> +	}
> +
> +	if (nframes > dtp->dt_options[DTRACEOPT_MAXFRAMES])
> +		nframes = dtp->dt_options[DTRACEOPT_MAXFRAMES];
> +
> +	if (arg1 != NULL) {
> +		if (arg1->dn_kind != DT_NODE_INT ||
> +		    ((arg1->dn_flags & DT_NF_SIGNED) &&
> +		    (int64_t)arg1->dn_value < 0))
> +			dnerror(arg1, D_USTACK_STRSIZE, "ustack( ) argument #2 "
> +				"must be a positive integer constant\n");
> +
> +		strsize = arg1->dn_value;
> +	}
> +
> +	return DTRACE_USTACK_ARG(nframes, strsize);
> +}
> +
>  static void
>  dt_cg_act_stack(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
>  {
> @@ -2378,42 +2414,6 @@ dt_cg_act_trunc(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
>  	dt_cg_store_val(pcb, trunc, DTRACEACT_LIBACT, NULL, DT_ACT_TRUNC);
>  }
>  
> -static uint64_t
> -dt_cg_ustack_arg(dtrace_hdl_t *dtp, dt_node_t *dnp)
> -{
> -	int		nframes = dtp->dt_options[DTRACEOPT_USTACKFRAMES];
> -	int		strsize = 0;
> -	dt_node_t	*arg0 = dnp->dn_args;
> -	dt_node_t	*arg1 = arg0 != NULL ? arg0->dn_list : NULL;
> -
> -	if (nframes == DTRACEOPT_UNSET)
> -		nframes = _dtrace_ustackframes;
> -
> -	if (arg0 != NULL) {
> -		if (!dt_node_is_posconst(arg0))
> -			dnerror(arg0, D_USTACK_FRAMES, "ustack( ) argument #1 "
> -				"must be a non-zero positive integer "
> -				"constant\n");
> -
> -		nframes = arg0->dn_value;
> -	}
> -
> -	if (nframes > dtp->dt_options[DTRACEOPT_MAXFRAMES])
> -		nframes = dtp->dt_options[DTRACEOPT_MAXFRAMES];
> -
> -	if (arg1 != NULL) {
> -		if (arg1->dn_kind != DT_NODE_INT ||
> -		    ((arg1->dn_flags & DT_NF_SIGNED) &&
> -		    (int64_t)arg1->dn_value < 0))
> -			dnerror(arg1, D_USTACK_STRSIZE, "ustack( ) argument #2 "
> -				"must be a positive integer constant\n");
> -
> -		strsize = arg1->dn_value;
> -	}
> -
> -	return DTRACE_USTACK_ARG(nframes, strsize);
> -}
> -
>  static void
>  dt_cg_act_ustack(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
>  {
> -- 
> 2.18.4
> 
>