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?
