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

[Kris Van Hees]

On Wed, Sep 07, 2022 at 03:32:33PM -0400, Eugene Loh via DTrace-devel wrote:
> 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.

I re-ordered the patches so that setopt is implemented before we add the
xfail marker to tst.keysort.d (and deleted that addition) to avoid the whole
situation.

> 
> > 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;
> >   			}
> >   		}
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel