Re: [PATCH] dtprobed: handle multiple providers in a single piece of DOF

[Eugene Loh <eugene.loh@oracle.com>]

On 11/14/24 15:38, Nick Alcock wrote:

> On 14 Nov 2024, Eugene Loh told this:
>
>> On 11/14/24 11:03, Nick Alcock wrote:
>>> diff --git a/test/unittest/usdt/tst.multiprovider.r.p b/test/unittest/usdt/tst.multiprovider.r.p
>>> new file mode 100755
>>> index 0000000000000..ae8493e3d5be4
>>> --- /dev/null
>>> +++ b/test/unittest/usdt/tst.multiprovider.r.p
>>> @@ -0,0 +1,2 @@
>>> +#!/bin/sh
>>> +grep -v '^ *ID' | sed 's,^[0-9]*,ID,; s,prov\(.\)[0-9]*,prov\1PID,; s,  *, ,g'
>> Okay, though the "grep -v" would be clearer to me if it checked  '^ *ID *PROVIDER *MODULE *FUNCTION *NAME$'.
> Why? We're not dependent on the format of the header, we just want to
> get rid of it.

It just seemed to me easier to figure out what was going on.  It looked 
like a recognizable header.

What do you think of ' *PROVIDER *'?  The 'ID' is short and strikes me 
as especially confusing if one looks at the post-processed output, which 
has 'ID' all over the place.

> Having the test fail because the header we don't care
> about changed layout seems wrong.
>
> I might just drop the first line with tail -n +1...
>
>> This whole thing would be clearer to me as a single awk, but maybe that's just me.
> How? Piles of gmatches?

gsub() or something.  Anyhow, your choice.

>> Comments might be nice.  I'm not sure how comfortably people read regex...  I need to concentrate hard to figure it out.
> Honestly this is just doing some "replace changing numbers" stuff like a
> hundred other .p's in the testsuite. Looking at the .r makes it obvious
> what it's up to.

No big deal either way (e.g., adding one-line comment).

>>> diff --git a/test/unittest/usdt/tst.multiprovider.sh b/test/unittest/usdt/tst.multiprovider.sh
>>> new file mode 100755
>>> index 0000000000000..d5b72c2be77ec
>>> --- /dev/null
>>> +++ b/test/unittest/usdt/tst.multiprovider.sh
>>> @@ -0,0 +1,15 @@
>>> +#!/bin/bash
>>> +#
>>> +# Oracle Linux DTrace.
>>> +# Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
>>> +# Licensed under the Universal Permissive License v 1.0 as shown at
>>> +# http://oss.oracle.com/licenses/upl.
>>> +#
>>> +if [ $# != 1 ]; then
>>> +	echo expected one argument: '<'dtrace-path'>'
>>> +	exit 2
>>> +fi
>>> +
>>> +dtrace=$1
>>> +
>>> +exec $dtrace $dt_flags -l -P 'prov*' -c `pwd`/test/triggers/usdt-tst-multiprovider
>> Okay, but there should also be a test that actually fires probes.
> ... why? What we care about is that DTrace can see the probes properly
> (since we already know from other tests that once it can do that,
> everything else works), and -l is an easy way to verify that (and in
> general exercise the entire pathway up to USDT probe discovery in dtrace
> itself) without having to get tangled up in even *more* possible bugs
> around probe firing. I hit enough bugs in the parser writing this as it
> was...
>
> I mean I can write one but I don't see what it's going to tell us.

I'm less concerned about the bugs we can foresee.  I'm more concerned 
about the bugs we don't foresee.

I'm even interested in the case where probes in different providers have 
the same name.

Different subject:  do we have a test for "dtrace -lv" on USDT typed args?