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

[Nick Alcock]

On 7 Sep 2022, Kris Van Hees spake thusly:

>> > +	if (val == NULL)
>> > +		val = dt_node_int(0);
>> 
>> So this defaults VAL to 0...
>
> Well, no.  This ensures we have a NULL val argument node so that the rest of
> the code can proceed without needing to deal with val as a NULL pointer.

Aha, this is a *string*. So 0 == NULL, right, ok.

>> >@@ -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);
>> > +
>> > +				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))
>> > +					val = data + vrec->dtrd_offset;
>> > +				else
>> > +					val = "1";
>> 
>> ... and then this, uh, no I have absolutely no idea what this is doing.
>> I guess future patches will put larger things into the value than a
>> dt_node_int? (But then why nail a 1 in there rather than what the user
>> passed in when something integral-or-smaller is passed? Why 1 at all,
>> rather than the 0 up above? Doesn't this deserve a comment?)
>
> The setopt machinery expects the option name and value to be strings.  So the
> size of the value (which is described in vrec) will be > sizeof(uint64_t) if
> there is a value passed,

Really? "a" is < sizeof(uint64_t). So is "foo" for that matter. This
seems like an... unreliable way to detect if a value was passed.

I feel like I'm missing something obvious :)

-- 
NULL && (void)