Re: [PATCH bpf-next v6 7/9] selftests/bpf: extract BPF skeleton generation into a helper script

> diff --git a/tools/testing/selftests/bpf/gen_bpf_skel.sh b/tools/testing/selftests/bpf/gen_bpf_skel.sh
> new file mode 100755
> index 000000000000..a752cb78c139
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/gen_bpf_skel.sh
> @@ -0,0 +1,93 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0

[ ... ]

> +# The bpftool binary is taken from $BPFTOOL (default: bpftool from PATH).
> +# On failure all outputs and intermediates are removed and the script
> +# exits non-zero; permissive-mode skipping is the caller's business
> +# (see skip_on_fail in the Makefile).
> +
> +set -u
> +
> +bpftool=${BPFTOOL:-bpftool}
> +name='' skel='' subskel=''
> +lskel=0 sign=0

[ ... ]

> +base=${skel%.skel.h}
> +base=${base%.lskel.h}
> +t1=$base.${infix}1.o
> +t2=$base.${infix}2.o
> +t3=$base.${infix}3.o
> +
> +fail() {
> +	rm -f "$skel" ${subskel:+"$subskel"} "$t1" "$t2" "$t3"
> +	exit 1
> +}
> +
> +"$bpftool" gen object "$t1" "$@" || fail
> +"$bpftool" gen object "$t2" "$t1" || fail
> +"$bpftool" gen object "$t3" "$t2" || fail
> +if ! cmp -s "$t2" "$t3"; then
> +	echo "$0: bpftool gen object is not deterministic for $skel" >&2
> +	fail
> +fi
> +
> +args=()
> +if [ "$sign" -eq 1 ]; then
> +	args+=(-S -k "${PRIVATE_KEY:?}" -i "${VERIFICATION_CERT:?}")
> +fi

The script's header comment promises that all outputs and intermediates
are removed on failure. Does this hold when PRIVATE_KEY or
VERIFICATION_CERT is unset?

The :? expansion terminates the shell immediately and the fail() cleanup
never runs, leaving t1, t2, and t3 behind. This isn't reachable from the
Makefile - which always exports both variables - but it breaks the
contract for direct invocations. Something like this at the top would
catch both up front:

if [ "$sign" -eq 1 ] && { [ -z "${PRIVATE_KEY-}" ] || [ -z "${VERIFICATION_CERT-}" ]; }; then
	echo "$0: --sign requires PRIVATE_KEY and VERIFICATION_CERT" >&2
	exit 1
fi

[ ... ]

> The distinct linked/llinked intermediate infixes are kept but derived
> inside the script: tests listed in LSKELS_EXTRA generate both a
> .skel.h and a .lskel.h from the same .bpf.o, and distinct intermediate
> names are what keeps parallel builds from racing.

The changelog says the intermediate names are "kept" and closes with
"behavior is unchanged", but the intermediate file stem changed for three
of the four rules.

The script derives the stem from the output header:

    base=${skel%.skel.h}
    base=${base%.lskel.h}
    t1=$base.${infix}1.o

The pre-image derived it from the input object for SKELS, LSKELS and
LSKELS_SIGNED, e.g. "$(<:.o=.linked1.o)" where $< is out/foo.bpf.o, so:

    SKELS:         out/foo.bpf.linked1.o   -> out/foo.linked1.o
    LSKELS:        out/foo.bpf.llinked1.o  -> out/foo.llinked1.o
    LSKELS_SIGNED: out/foo.bpf.llinked1.o  -> out/foo.llinked1.o
    SKELS_LINKED:  out/foo.linked1.o       -> unchanged

The infixes themselves are indeed preserved. I found no functional
consequence - no stem collision, nothing references the *.linked[123].o
names, and EXTRA_CLEAN's *.o glob covers both spellings.

Should the changelog note the change, since it specifically discusses
these names and the anti-racing property that depends on them?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32610372822