Hi David, Thank you for the review!
On 17/02/2026 12:29, David Laight wrote: > On Mon, 16 Feb 2026 17:30:34 +0100 > "Matthieu Baerts (NGI0)" <[email protected]> wrote: > >> The verification signature header generation requires converting a >> binary certificate to a C array. Previously this only worked with xxd, >> and a switch to hexdump has been done in commit b640d556a2b3 >> ("selftests/bpf: Remove xxd util dependency"). >> >> hexdump is a more common utility program, yet it might not be installed >> by default. When it is not installed, BPF selftests build without >> errors, but tests_progs is unusable: it exits with the 255 code and >> without any error messages. When manually reproducing the issue, it is >> not too hard to find out that the generated verification_cert.h file is >> incorrect, but that's time consuming. When digging the BPF selftests >> build logs, this line can be seen amongst thousands others, but ignored: >> >> /bin/sh: 2: hexdump: not found >> >> Here, od is used with awk, instead of hexdump with sed. od is coming >> from the core utils package, and this new od command produces the same >> output when using od from GNU coreutils, uutils, and even busybox. This >> is more portable, and it produces the same results as what was done >> before with hexdump (without trailing whitespaces as a bonus). >> >> Fixes: b640d556a2b3 ("selftests/bpf: Remove xxd util dependency") >> Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> >> --- >> Originally, I was going to add a check to stop the build if hexdump was >> not available, but switching to 'od' seems to be a better solution while >> not adding a new dependency. >> >> Because test_progs was not reporting why it became unusable, I added a >> Fixes tag to have this backported, to help others. Feel free to remove >> it, or even drop the patch if you prefer to stick with hexdump. >> --- >> tools/testing/selftests/bpf/Makefile | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tools/testing/selftests/bpf/Makefile >> b/tools/testing/selftests/bpf/Makefile >> index c6bf4dfb1495..5a618d14243e 100644 >> --- a/tools/testing/selftests/bpf/Makefile >> +++ b/tools/testing/selftests/bpf/Makefile >> @@ -723,7 +723,7 @@ $(VERIFICATION_CERT) $(PRIVATE_KEY): $(VERIFY_SIG_SETUP) >> # Generates a header with C array declaration, containing >> test_progs_verification_cert bytes >> $(VERIFY_SIG_HDR): $(VERIFICATION_CERT) >> $(Q)(echo "unsigned char test_progs_verification_cert[] = {"; \ >> - hexdump -v -e '12/1 " 0x%02x," "\n"' $< | sed 's/0x ,//g; >> $$s/,$$//'; \ >> + od -v -t 'xC' -w12 $< | awk 'NF > 1 {for (i=2; i<=NF; i++) { printf " >> 0x%s,", $$i }; printf "\n"}'; \ > > That is subtly different. > The hexdump version deletes the final ',' from the last full line. > But not a partial line because there are trailing spaces (from before > the '0x ,' that get removed). Good catch! > That may not matter, it is valid C, but not C++ (unless they've changed > the rules recently). Is it an issue here? From what I see, this verification_cert.h file is only used in test_progs.c. > od could be used with: > sed -e 's/ /, 0x/g; s/$/,/; s/^[0-7]*,//' > which outputs an empty line as well as the trailing ','. If adding an extra empty line is not a "visual" issue, I'm happy to change to this shorter sed command if preferred! > Both removable with a more complex command line. Indeed. But if they don't cause issues, no need to remove them, right? Cheers, Matt -- Sponsored by the NGI0 Core fund.
