[DTrace-devel] [PATCH 1/5] Implement setopt()

[Eugene Loh]

On 9/6/22 21:36, Kris Van Hees via DTrace-devel wrote:

> The basic mechanics of the setopt() action are implemented in this
> patch.  Each dynamic runtime options that can be set using setopt()
> must provide support for it.

There ought to be at least one test, and if rudimentary setopt() support 
was the only thing only keeping tst.keysort.d from passing then that 
could be the test.

> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
>   libdtrace/dt_cg.c      | 11 ++++++++++
>   libdtrace/dt_consume.c | 47 +++++++++++++++++++++++++++++++++---------
>   2 files changed, 48 insertions(+), 10 deletions(-)
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index cb51c474..ffab2443 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -1751,6 +1751,17 @@ dt_cg_act_raise(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
>   static void
>   dt_cg_act_setopt(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
>   {
> +	dt_node_t	*opt = dnp->dn_args;
> +	dt_node_t	*val = opt->dn_list;
> +
> +	TRACE_REGSET("setopt(): Begin ");
> +	dt_cg_store_val(pcb, opt, DTRACEACT_LIBACT, NULL, DT_ACT_SETOPT);
> +
> +	/* If not value is given, we default to 0. */
> +	if (val == NULL)
> +		val = dt_node_int(0);

I don't get it.  Why int(0) vs int(1) vs not writing a second value 
altogether?

> +	dt_cg_store_val(pcb, val, DTRACEACT_LIBACT, NULL, DT_ACT_SETOPT);
> +	TRACE_REGSET("setopt(): End   ");
>   }
>   
>   /*
> diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
> index 44d7e935..011653d8 100644
> --- a/libdtrace/dt_consume.c
> +++ b/libdtrace/dt_consume.c
> @@ -2150,6 +2150,7 @@ dt_consume_one_probe(dtrace_hdl_t *dtp, FILE *fp, char *data, uint32_t size,
>   		     void *arg)
>   {
>   	dtrace_epid_t		epid;
> +	dtrace_datadesc_t	*epd = pdat->dtpda_ddesc;

How about just "dtrace_datadesc_t *epd;"?  The value you write here is 
going to get overwritten anyhow.

>   	dt_spec_buf_t		tmpl;
>   	dt_spec_buf_t		*dtsb;
>   	int			specid;
> @@ -2170,11 +2171,12 @@ dt_consume_one_probe(dtrace_hdl_t *dtp, FILE *fp, char *data, uint32_t size,
>   	pdat->dtpda_data = data;
>   
>   	rval = dt_epid_lookup(dtp, epid, &pdat->dtpda_ddesc,
> -			      &pdat->dtpda_pdesc);
> +					 &pdat->dtpda_pdesc);
>   	if (rval != 0)
>   		return dt_set_errno(dtp, EDT_BADEPID);
>   
> -	if (pdat->dtpda_ddesc->dtdd_uarg != DT_ECB_DEFAULT) {
> +	epd = pdat->dtpda_ddesc;
> +	if (epd->dtdd_uarg != DT_ECB_DEFAULT) {
>   		rval = dt_handle(dtp, pdat);
>   
>   		if (rval == DTRACE_CONSUME_NEXT)
> @@ -2332,6 +2334,31 @@ dt_consume_one_probe(dtrace_hdl_t *dtp, FILE *fp, char *data, uint32_t size,
>   				ftruncate(fileno(fp), 0);
>   				fseeko(fp, 0, SEEK_SET);
>   
> +				continue;
> +			case DT_ACT_SETOPT: {
> +				caddr_t			opt = recdata;
> +				caddr_t			val;
> +				dtrace_recdesc_t	*vrec;
> +
> +				if (i == epd->dtdd_nrecs - 1)
> +					return dt_set_errno(dtp, EDT_BADSETOPT);

Or if dt_cg.c doesn't write a val arg -- that is, if i == 
epd->dtdd_nrecs-1 -- then just ignore the vrec stuff and call 
dt_setopt(dtp,pdat,opt,"set").

> +
> +				vrec = &epd->dtdd_recs[++i];
> +				if (vrec->dtrd_action != act &&
> +				    vrec->dtrd_arg != rec->dtrd_arg)
> +					return dt_set_errno(dtp, EDT_BADSETOPT);
> +
> +				if (vrec->dtrd_size > sizeof(uint64_t))

So if strsize is small, then the value will be ignored?

> +					val = data + vrec->dtrd_offset;
> +				else
> +					val = "1";
> +
> +				if (dt_setopt(dtp, pdat, opt, val) != 0)
> +					return DTRACE_WORKSTATUS_ERROR;
> +
> +				continue;
> +			}
> +			default:
>   				continue;
>   			}
>   		}