On 6/3/19 2:59 PM, Markus Armbruster wrote: > Laszlo Ersek <ler...@redhat.com> writes: > >> Hi Markus, >> >> (sorry about the late reply, I've been away.) >> >> On 05/28/19 20:12, Markus Armbruster wrote: >> >>> EDK2 Firmware >>> M: Laszlo Ersek <ler...@redhat.com> >>> M: Philippe Mathieu-Daudé <phi...@redhat.com> >>> tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h >> >> This header file does have a multiple inclusion guard: >> >>> /** @file >>> Expose the address(es) of the ACPI RSD PTR table(s) and the SMBIOS entry >>> point(s) in a MB-aligned structure to the hypervisor. >>> >>> [...] >>> **/ >>> >>> #ifndef __BIOS_TABLES_TEST_H__ >>> #define __BIOS_TABLES_TEST_H__ >>> >>> [...] >>> >>> #endif // __BIOS_TABLES_TEST_H__ >> >> It's possible that "scripts/clean-header-guards.pl" does not recognize >> the guard. > > Correct. I fixed the script in my tree. > >> According to the ISO C standard, "All identifiers that begin with an >> underscore and either an uppercase letter or another underscore are >> always reserved for any use". Therefore, technically speaking, the above >> inclusion guard implies undefined behavior. In practice, this particular >> style for header guards is extremely common in the edk2 codebase: >> >> $ git grep '^#ifndef __' -- '*.h' | wc -l >> 1012 >> >> And, "tests/uefi-test-tools/UefiTestToolsPkg" follows the edk2 coding >> style. >> >> That said, if you'd like to remove the leading "__" from the macro name, >> I'd be fully OK with that. > > We routinely exempt files from style cleanups when we have a reason. If > you want this one to be exempted, that's fine with me. > > If we decide not to exempt it, then I want a header guard that makes my > (fixed) script happy. It isn't right now: > > $ scripts/clean-header-guards.pl -nv > tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h > tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h > guard __BIOS_TABLES_TEST_H__ needs cleanup: > is a reserved identifier, doesn't end with _H, doesn't match the file > name > [...] > > Removing the leading "__" takes care of the first complaint: > > tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h > guard BIOS_TABLES_TEST_H__ needs cleanup: > doesn't end with _H, doesn't match the file name > > Removing the trailing "__" as well takes care of the second one: > > tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h > guard BIOS_TABLES_TEST_H needs cleanup: > doesn't match the file name > > To get rid of the last one, we can: > > * Rename to BIOSTABLESTEST_H. Easy. > > * Teach scripts/clean-header-guards.pl to capitalize StudlyCaps > filenames to STUDLY_CAPS rather than STUDLYCAPS. But that would break > include/libdecnumber/*.h. > > * Teach scripts/clean-header-guards to accept either STUDLYCAPS or > STUDLY_CAPS. Considering we have exactly one filename that needs > this, I'd prefer not to. > > My first preference is BIOSTABLESTEST_H, second is to exempt the file. > Yours? >
What about excluding UefiTestToolsPkg? $ git grep '^#ifndef __' -- \ '*.h' ':!tests/uefi-test-tools/UefiTestToolsPkg'