Re: [edk2-devel] [PATCH v2 1/1] BaseTools: Align include guards policy

["Yuwei Chen" <yuwei.chen@intel.com>]

Hi Pierre,

There seems already have a Bugzilla link for this issue: https://bugzilla.tianocore.org/show_bug.cgi?id=3094
And a personally concern: Some of the current codes still use "_***_H_", such as " __PEI_APRIORI_FILE_NAME_H__ ". If the ECC check only support the coding standard you mentioned, will we need to change all these codes? Or should ECC check support the origin format too?

Regards,
Yuwei (Christine) 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> PierreGondois
> Sent: Tuesday, February 16, 2021 5:29 PM
> To: devel@edk2.groups.io; Feng, Bob C <bob.c.feng@intel.com>;
> gaoliming@byosoft.com.cn; rebecca@nuviainc.com;
> sami.mujawar@arm.com
> Cc: leif@nuviainc.com
> Subject: [edk2-devel] [PATCH v2 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>
> Reviewed-by: Sami Mujawar <Sami.Mujawar@arm.com>
> ---
> The changes can be seen at:
> https://github.com/PierreARM/edk2/tree/1619_Ecc_BaseTools_include_gua
> rds_v2
> 
> Notes:
>     v2:
>      - Place new copyright on top of old ones [Rebecca]
> 
>  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..7a012617fd35 100644
> --- a/BaseTools/Source/Python/Ecc/Check.py
> +++ b/BaseTools/Source/Python/Ecc/Check.py
> @@ -1,6 +1,7 @@
>  ## @file
>  # This file is used to define checkpoints used by ECC tool  #
> +# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
>  # Copyright (c) 2008 - 2020, Intel Corporation. All rights reserved.<BR>  #
> SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -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..d97bf7948ce8 100644
> --- a/BaseTools/Source/Python/Ecc/EccToolError.py
> +++ b/BaseTools/Source/Python/Ecc/EccToolError.py
> @@ -1,6 +1,7 @@
>  ## @file
>  # Standardized Error Handling infrastructures.
>  #
> +# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
>  # Copyright (c) 2008 - 2018, Intel Corporation. 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
> 
> 
> 
> 
>