On 11/29/23 18:47, Christopher Ferris wrote: > This patch fixes a bug in the handling of section flags. > > I added a new test elf file that shows the failures, but I'm not sure if you > would prefer some other way of doing the testing.
Applied, but no I don't like the test. It currently fails on test_host: $ TEST_HOST=1 make test_readelf scripts/test.sh readelf PASS: readelf -h PASS: readelf -S FAIL: readelf -S flags echo -ne '' | readelf -SW /home/landley/toybox/toybox/tests/files/elf/ndk-elf-note-shflags | head -32 Design-wise calling for an exact match for a very large block of english text is brittle. If spacing changes, the test breaks. If somebody rephrases "Number of section headers:" the test breaks. If the output entries are reordered (including adding more entries in the middle) the test breaks. Even if we fiddle to match debian's output, version skew will break us in a year or two... It also doesn't specifically test what it fixes, and it's unclear what such a broad test is checking for. There's no way I could work out from the test that the patch adding it did: - for (j=0; j<12; j++) if (s.flags&(1<<j)) *p++ = "WAXxMSILOTC"[j]; + for (j=0; j<12; j++) if (s.flags&(1<<j)) *p++ = "WAXxMSILOGTC"[j]; Lemme think a bit. Possibly there's missing test infrastructure to make this sort of thing easier, some sort of automatic regex match maybe? (Like I added to txpect already in commit 8c7af93bde17 but that's an expect style call-and-response test infrastructure, not the "run this, what was the output" style most tests use...) Thanks, Rob _______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net