[DTrace-devel] [PATCH 1/5] Implement setopt()
Eugene Loh
eugene.loh at oracle.com
Wed Sep 7 19:22:26 UTC 2022
On 9/7/22 11:04, Kris Van Hees via DTrace-devel wrote:
> On Wed, Sep 07, 2022 at 03:50:31PM +0100, Nick Alcock wrote:
>> On 7 Sep 2022, Kris Van Hees via DTrace-devel told this:
>>
>>> The basic mechanics of the setopt() action are implemented in this
>>> patch. Each dynamic runtime options that can be set using setopt()
>> s/options/option/
> Thanks.
>
>>> 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. */
>> s/not/no/
> Thanks.
>
>>> + 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.
>
>>> @@ -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, and == sizeof(uint64_t) if no value was specified
> (e.g. for toggle options) since then a literal 0 is passed.
What if strsize is very small? Wouldn't that break this logic?
More information about the DTrace-devel
mailing list