Re: [edk2-devel] 回复: [PATCH v1 1/1] BaseTools: Align include guards policy

["Leif Lindholm" <leif@nuviainc.com>]

Hi Liming,

If it was purely a question of style, I would agree that whatever is
70% used should be the norm. But this is not really an issue under our
control.

Macros starting with leading _ are reserved for toolchain use.
Some toolchains, i.e. clang, have dedicated warnings for this.

Whether we want to enforce this lazily (prevent new additions, change
existing ones on rename) or with an all-out search-replace is a
different question.

Either way, this patch sounds like a useful change.
Adding the check for the end of the string would also help improving
code consistency.

/
    Leif

On Tue, Jan 26, 2021 at 09:22:06 +0800, gaoliming wrote:
> Pierre:
>   There are some discussion on the syntax of the header file macro. I
> suggest we align the syntax first, then add this checker in ECC tool. 
> 
>   In MdePkg, there are 555 header files. 70% header files use the style
> __BASE_H__ as the file header macro. Others use the style _BTT_H_. 
> 
>   For this case, I would propose to update EDK II C Coding Standards
> Specification to align the code. 
>  
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: Pierre.Gondois@arm.com <Pierre.Gondois@arm.com>
> > 发送时间: 2021年1月25日 23:45
> > 收件人: devel@edk2.groups.io; bob.c.feng@intel.com;
> > gaoliming@byosoft.com.cn
> > 抄送: sami.mujawar@arm.com
> > 主题: [PATCH v1 1/1] BaseTools: Align include guards policy
> > 
> > From: Pierre Gondois <Pierre.Gondois@arm.com>
> > 
> > The EDK II C Coding Standards Specification states that:
> > "Names starting with one or two underscores, such as
> > _MACRO_GUARD_FILE_NAME_H_, must not be used. They are
> > reserved for compiler implementation." [1]
> > 
> > The Ecc tool currently checks that the include guard end with
> > a trailing underscore. Thus, the check and the error message
> > should both be modified.
> > 
> > The new check forces having one sole trailing underscore
> > character, as the example in the specification shows:
> > "FILE_NAME_H_" [1]
> > This would allow to have more consistency.
> > 
> > [1] Section 5.3.5 "All include file contents must be protected
> > by a #include guard":
> > https://edk2-docs.gitbook.io/
> > edk-ii-c-coding-standards-specification/
> > 5_source_files/53_include_files
> > 
> > Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> > ---
> >  BaseTools/Source/Python/Ecc/Check.py        | 3 ++-
> >  BaseTools/Source/Python/Ecc/EccToolError.py | 3 ++-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/BaseTools/Source/Python/Ecc/Check.py
> > b/BaseTools/Source/Python/Ecc/Check.py
> > index 6087abfa4d8d..14759d21f5d8 100644
> > --- a/BaseTools/Source/Python/Ecc/Check.py
> > +++ b/BaseTools/Source/Python/Ecc/Check.py
> > @@ -2,6 +2,7 @@
> >  # This file is used to define checkpoints used by ECC tool
> >  #
> >  # Copyright (c) 2008 - 2020, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
> >  # SPDX-License-Identifier: BSD-2-Clause-Patent
> >  #
> >  from __future__ import absolute_import
> > @@ -1438,7 +1439,7 @@ class Check(object):
> >              RecordSet = EccGlobalData.gDb.TblFile.Exec(SqlCommand)
> >              for Record in RecordSet:
> >                  Name = Record[1].replace('#ifndef', '').strip()
> > -                if Name[-1] != '_':
> > +                if Name[0] == '_' or Name[-1] != '_' or Name[-2] == '_':
> >                      if not
> > EccGlobalData.gException.IsException(ERROR_NAMING_CONVENTION_CHE
> > CK_IFNDEF_STATEMENT, Name):
> > 
> > EccGlobalData.gDb.TblReport.Insert(ERROR_NAMING_CONVENTION_CHECK
> > _IFNDEF_STATEMENT, OtherMsg="The #ifndef name [%s] does not follow the
> > rules" % (Name), BelongsToTable=FileTable, BelongsToItem=Record[0])
> > 
> > diff --git a/BaseTools/Source/Python/Ecc/EccToolError.py
> > b/BaseTools/Source/Python/Ecc/EccToolError.py
> > index 0ff3b42674d4..58d0749477b2 100644
> > --- a/BaseTools/Source/Python/Ecc/EccToolError.py
> > +++ b/BaseTools/Source/Python/Ecc/EccToolError.py
> > @@ -2,6 +2,7 @@
> >  # Standardized Error Handling infrastructures.
> >  #
> >  # Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
> >  # SPDX-License-Identifier: BSD-2-Clause-Patent
> >  #
> > 
> > @@ -161,7 +162,7 @@ gEccErrorMessage = {
> >      ERROR_NAMING_CONVENTION_CHECK_ALL : "",
> >      ERROR_NAMING_CONVENTION_CHECK_DEFINE_STATEMENT : "Only
> > capital letters are allowed to be used for #define declarations",
> >      ERROR_NAMING_CONVENTION_CHECK_TYPEDEF_STATEMENT : "Only
> > capital letters are allowed to be used for typedef declarations",
> > -    ERROR_NAMING_CONVENTION_CHECK_IFNDEF_STATEMENT : "The
> > #ifndef at the start of an include file should use both prefix and postfix
> > underscore characters, '_'",
> > +    ERROR_NAMING_CONVENTION_CHECK_IFNDEF_STATEMENT : "The
> > #ifndef at the start of an include file should have one postfix
> underscore, and
> > no prefix underscore character '_'",
> >      ERROR_NAMING_CONVENTION_CHECK_PATH_NAME : """Path name
> > does not follow the rules: 1. First character should be upper case 2. Must
> > contain lower case characters 3. No white space characters""",
> >      ERROR_NAMING_CONVENTION_CHECK_VARIABLE_NAME :
> > """Variable name does not follow the rules: 1. First character should be
> upper
> > case 2. Must contain lower case characters 3. No white space characters 4.
> > Global variable name must start with a 'g'""",
> >      ERROR_NAMING_CONVENTION_CHECK_FUNCTION_NAME :
> > """Function name does not follow the rules: 1. First character should be
> upper
> > case 2. Must contain lower case characters 3. No white space
> characters""",
> > --
> > 2.17.1
> 
> 
> 
> 
> 
> 
> 
>