Re: [Qemu-devel] Headers without multiple inclusion guards

[Markus Armbruster]

Laszlo Ersek <address@hidden> 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 <address@hidden>
>> M: Philippe Mathieu-Daudé <address@hidden>
>> 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?