[DTrace-devel] [PATCH v2 2/3] usdt parser: handle encoded hyphens

[Eugene Loh]

Reviewed-by: Eugene Loh <eugene.loh at oracle.com>

Little nits below to consider.

On 7/1/25 11:12, Kris Van Hees wrote:
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
>   libcommon/usdt_parser_notes.c             | 17 ++++++
>   test/unittest/usdt/tst.encoded_hyphens.r  |  0
>   test/unittest/usdt/tst.encoded_hyphens.sh | 74 +++++++++++++++++++++++
>   3 files changed, 91 insertions(+)
>   create mode 100644 test/unittest/usdt/tst.encoded_hyphens.r
>   create mode 100755 test/unittest/usdt/tst.encoded_hyphens.sh
>
> diff --git a/libcommon/usdt_parser_notes.c b/libcommon/usdt_parser_notes.c
> index fb57f119..d3d744fb 100644
> --- a/libcommon/usdt_parser_notes.c
> +++ b/libcommon/usdt_parser_notes.c
> @@ -471,6 +471,23 @@ parse_usdt_note(int out, dof_helper_t *dhp, usdt_data_t *data,
>   	}
>   	prbt.off = off;
>   
> +	/*
> +	 * If the probe name has encoded hyphens, perform in-place changing
> +	 * from "__" into "-".
> +	 */
> +	if (strstr(prbt.prb, "__") != NULL) {
> +		char		*q;
> +		const char	*s = prbt.prb, *e = p;
> +
> +		for (q = (char *)s; s < e; s++, q++) {
> +			if (s[0] == '_' && s[1] == '_') {
> +				*q = '-';
> +				s++;
> +			} else if (s > q)
> +				*q = *s;
> +		}
> +	}
> +
>   	if ((prp = dt_htab_lookup(prbmap, &prbt)) == NULL) {
>   		if ((prp = malloc(sizeof(dt_probe_t))) == NULL) {
>   			usdt_error(out, ENOMEM, "Failed to allocate probe");
> diff --git a/test/unittest/usdt/tst.encoded_hyphens.r b/test/unittest/usdt/tst.encoded_hyphens.r
> new file mode 100644
> index 00000000..e69de29b

Empty?  I guess that works.  I usually have something like "echo 
success" at the end of the script and "success" in the .r file as a 
sanity check that we got to the end of the script, but I admit I see 
nothing in this test I can think of where things could go wrong.  Up to you.

> diff --git a/test/unittest/usdt/tst.encoded_hyphens.sh b/test/unittest/usdt/tst.encoded_hyphens.sh
> new file mode 100755
> index 00000000..90608e25
> --- /dev/null
> +++ b/test/unittest/usdt/tst.encoded_hyphens.sh
> @@ -0,0 +1,74 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2025, 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
> +CFLAGS="$test_cppflags"
> +LDFLAGS="$test_ldflags"
> +
> +DIRNAME="$tmpdir/usdt-entryreturn.$$.$RANDOM"

s/entryreturn/encoded_hyphens/

> +mkdir -p $DIRNAME
> +cd $DIRNAME
> +
> +cat > test.c <<EOF
> +#include <sys/sdt.h>
> +
> +int
> +main(int argc, char **argv)
> +{
> +	DTRACE_PROBE(test_prov, entry);
> +	DTRACE_PROBE(test_prov, __entry);
> +	DTRACE_PROBE(test_prov, foo__entry);
> +	DTRACE_PROBE(test_prov, carpentry);
> +	DTRACE_PROBE(test_prov, miniatureturn);
> +	DTRACE_PROBE(test_prov, foo__return);
> +	DTRACE_PROBE(test_prov, __return);
> +	/*
> +	 * Unfortunately, a "return" probe is not currently possible due to
> +	 * the conflict with a reserved word.
> +	 */
> +	DTRACE_PROBE(test_prov, done);
> +}
> +EOF
> +
> +cat > prov.d <<EOF
> +provider test_prov {
> +	probe entry();
> +	probe __entry();
> +	probe foo__entry();
> +	probe carpentry();
> +	probe miniatureturn();
> +	probe foo__return();
> +	probe __return();
> +	probe done();
> +};
> +EOF
> +
> +${CC} ${CFLAGS} -c test.c
> +if [ $? -ne 0 ]; then
> +	echo "failed to compile test.c" >& 2
> +	exit 1
> +fi
> +$dtrace $dt_flags -G -s prov.d test.o
> +if [ $? -ne 0 ]; then
> +	echo "failed to create USDT data" >& 2
> +	exit 1
> +fi
> +${CC} ${LDFLAGS} -o test test.o prov.o
> +if [ $? -ne 0 ]; then
> +	echo "failed to link final executable" >& 2
> +	exit 1
> +fi
> +
> +./test
> +status=$?
> +
> +exit $status

Yeah.  On first read, I'm left wondering where the check is that "__" 
got turned into "-".  I guess the check is kind of implicit. Another way 
to reassure readers could be to add one more line ("$dtrace -c ./test 
-lP 'test_prov$target' |& gawk '/test_prov/ { print $NF}'") to the 
script and the corresponding output to the .r file.