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

[Eugene Loh]

On 9/7/22 16:54, Kris Van Hees via DTrace-devel wrote:

> On Wed, Sep 07, 2022 at 04:15:05PM -0400, Kris Van Hees via DTrace-devel wrote:
>> diff --git 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;
>>   	dt_spec_buf_t		tmpl;
>>   	dt_spec_buf_t		*dtsb;
>>   	int			specid;

Maybe I missed something, but I thought epd is set later, before its 
first use.  So epd should not be set in its declaration.

>> @@ -2332,6 +2334,37 @@ 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);
>> +
>> +				/*
>> +				 * Two possibilities: either a string was
>> +				 * passed (the option value), or a uint64_t
>> +				 * was passed (representing NULL, i.e. no value
>> +				 * was given).
>> +				 */
>> +				if (vrec->dtrd_size > sizeof(uint64_t))
>> +					val = data + vrec->dtrd_offset;
>> +				else
>> +					val = "1";
> Eugene brings up a valid point (that was even a problem on v1 I expect) - one
> could set the strsize to a ridiculous size and this might fail.  But I *can*
> validly check the alignment of the data record.  If it is 1, we have a string.
> If it is > 1 then we have something else which thereby indicates this is a
> toggle (option without value).
>
> I will make that change because even if something prevents the strsize to be
> set less than 8, I still agree it far from ideal.

Granted, it's an odd fix, but it should work just fine.  But then maybe 
also update the comment in dt_cg.c that there is nothing magical about 
using dt_node_int(0).  All that is needed is to store anything with 
non-unit alignment.

>> +
>> +				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