[PATCH v1 1/1] BaseTools: Align include guards policy

[PATCH v1 1/1] BaseTools: Align include guards policy

[PierreGondois]

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_CHECK_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

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

[PierreGondois]

[-- Attachment #1: Type: text/plain, Size: 736 bytes --]

Please consider that currently, a vast majority of include guards have one or two leading underscore (such as __INCLUDE_FILE_H_) and are not respecting the edk2 coding standard. This patch will raise many Ecc errors in the CI. They should however not break it. Indeed, the Ecc errors should only reject a patch if it modifies a line having an Ecc error. This means that:
- merging this patch should not break all the CI jobs
- modifying a header file that has a bad include guard will make the CI prevent the patch from being merged
- any newly created header file will trigger the new check and ask for a correction
- any modification of a bad include guard will trigger the new check and ask for a correction

Regards,
Pierre

[-- Attachment #2: Type: text/html, Size: 764 bytes --]

回复: [PATCH v1 1/1] BaseTools: Align include guards policy

[gaoliming]

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

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

[Leif Lindholm]

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
> 
> 
> 
> 
> 
> 
> 
> 

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

[PierreGondois]

Hello,
I was wondering if there had been an agreement on how to proceed.
For reference, the first messages are at:
https://edk2.groups.io/g/devel/topic/80106488

Regards,
Pierre


From: Leif Lindholm <leif@nuviainc.com>
Sent: Tuesday, January 26, 2021 11:07 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>; gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>
Cc: Pierre Gondois <Pierre.Gondois@arm.com>; bob.c.feng@intel.com <bob.c.feng@intel.com>; Sami Mujawar <Sami.Mujawar@arm.com>; 'Laszlo Ersek' <lersek@redhat.com>; 'Michael D Kinney' <michael.d.kinney@intel.com>; 'Andrew Fish' <afish@apple.com>
Subject: Re: [edk2-devel] 回复: [PATCH v1 1/1] BaseTools: Align include guards policy

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
>
>
>
>
>
> 
>
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

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

[gaoliming]

Pierre:
  I get Leif point. I also see clang warning https://clang.llvm.org/docs/DiagnosticsReference.html#wreserved-id-macro

  I agree the compiler may reserve the macro starting with _ prefix. But so far, I don't get any error report that the macro conflict with the compiler reserved macros. So, I don't think this change is urgent. 

  I would like to agree to add this ECC checker. But, the clean up to the existing coding can be planned later.

Thanks
Liming
> -----邮件原件-----
> 发件人: Pierre Gondois <Pierre.Gondois@arm.com>
> 发送时间: 2021年2月4日 6:32
> 收件人: Leif Lindholm <leif@nuviainc.com>; devel@edk2.groups.io;
> gaoliming@byosoft.com.cn
> 抄送: bob.c.feng@intel.com; Sami Mujawar <Sami.Mujawar@arm.com>;
> 'Laszlo Ersek' <lersek@redhat.com>; 'Michael D Kinney'
> <michael.d.kinney@intel.com>; 'Andrew Fish' <afish@apple.com>
> 主题: Re: [edk2-devel] 回复: [PATCH v1 1/1] BaseTools: Align include guards
> policy
> 
> Hello,
> I was wondering if there had been an agreement on how to proceed.
> For reference, the first messages are at:
> https://edk2.groups.io/g/devel/topic/80106488
> 
> Regards,
> Pierre
> 
> 
> From: Leif Lindholm <leif@nuviainc.com>
> Sent: Tuesday, January 26, 2021 11:07 AM
> To: devel@edk2.groups.io <devel@edk2.groups.io>;
> gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>
> Cc: Pierre Gondois <Pierre.Gondois@arm.com>; bob.c.feng@intel.com
> <bob.c.feng@intel.com>; Sami Mujawar <Sami.Mujawar@arm.com>; 'Laszlo
> Ersek' <lersek@redhat.com>; 'Michael D Kinney'
> <michael.d.kinney@intel.com>; 'Andrew Fish' <afish@apple.com>
> Subject: Re: [edk2-devel] 回复: [PATCH v1 1/1] BaseTools: Align include
> guards policy
> 
> 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
> >
> >
> >
> >
> >
> > 
> >
> >
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended recipient,
> please notify the sender immediately and do not disclose the contents to any
> other person, use it for any purpose, or store or copy the information in any
> medium. Thank you.

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

[Sami Mujawar]

Hi Liming,

I agree the clean-up activity is going to be a huge task and can be done later. Maybe a script could help in this effort.
Also, this patch will prevent new code from deviating from the coding standard.

Regards,

Sami Mujawar

-----Original Message-----
From: gaoliming <gaoliming@byosoft.com.cn> 
Sent: 04 February 2021 07:28 AM
To: Pierre Gondois <Pierre.Gondois@arm.com>; 'Leif Lindholm' <leif@nuviainc.com>; devel@edk2.groups.io
Cc: bob.c.feng@intel.com; Sami Mujawar <Sami.Mujawar@arm.com>; 'Laszlo Ersek' <lersek@redhat.com>; 'Michael D Kinney' <michael.d.kinney@intel.com>; 'Andrew Fish' <afish@apple.com>
Subject: 回复: [edk2-devel] 回复: [PATCH v1 1/1] BaseTools: Align include guards policy

Pierre:
  I get Leif point. I also see clang warning https://clang.llvm.org/docs/DiagnosticsReference.html#wreserved-id-macro

  I agree the compiler may reserve the macro starting with _ prefix. But so far, I don't get any error report that the macro conflict with the compiler reserved macros. So, I don't think this change is urgent. 

  I would like to agree to add this ECC checker. But, the clean up to the existing coding can be planned later.

Thanks
Liming
> -----邮件原件-----
> 发件人: Pierre Gondois <Pierre.Gondois@arm.com>
> 发送时间: 2021年2月4日 6:32
> 收件人: Leif Lindholm <leif@nuviainc.com>; devel@edk2.groups.io;
> gaoliming@byosoft.com.cn
> 抄送: bob.c.feng@intel.com; Sami Mujawar <Sami.Mujawar@arm.com>;
> 'Laszlo Ersek' <lersek@redhat.com>; 'Michael D Kinney'
> <michael.d.kinney@intel.com>; 'Andrew Fish' <afish@apple.com>
> 主题: Re: [edk2-devel] 回复: [PATCH v1 1/1] BaseTools: Align include guards
> policy
> 
> Hello,
> I was wondering if there had been an agreement on how to proceed.
> For reference, the first messages are at:
> https://edk2.groups.io/g/devel/topic/80106488
> 
> Regards,
> Pierre
> 
> 
> From: Leif Lindholm <leif@nuviainc.com>
> Sent: Tuesday, January 26, 2021 11:07 AM
> To: devel@edk2.groups.io <devel@edk2.groups.io>;
> gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>
> Cc: Pierre Gondois <Pierre.Gondois@arm.com>; bob.c.feng@intel.com
> <bob.c.feng@intel.com>; Sami Mujawar <Sami.Mujawar@arm.com>; 'Laszlo
> Ersek' <lersek@redhat.com>; 'Michael D Kinney'
> <michael.d.kinney@intel.com>; 'Andrew Fish' <afish@apple.com>
> Subject: Re: [edk2-devel] 回复: [PATCH v1 1/1] BaseTools: Align include
> guards policy
> 
> 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
> >
> >
> >
> >
> >
> > 
> >
> >
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended recipient,
> please notify the sender immediately and do not disclose the contents to any
> other person, use it for any purpose, or store or copy the information in any
> medium. Thank you.

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

[PierreGondois]

[-- Attachment #1: Type: text/plain, Size: 149 bytes --]

Hi Liming,
I agree this is not an urgent change, but will there be a timeframe when non-essential clean-up is going to happen?

Regards,
Pierre

[-- Attachment #2: Type: text/html, Size: 165 bytes --]

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

[gaoliming]

[-- Attachment #1: Type: text/plain, Size: 696 bytes --]

Pierre:

 I agree to add this checker in ECC tool first. Then, we can start to discuss the topic about the code clean up on the early phase of next stable tag. 

 

Thanks

Liming

发件人: bounce+27952+71690+4905953+8761045@groups.io <bounce+27952+71690+4905953+8761045@groups.io> 代表 PierreGondois
发送时间: 2021年2月16日 6:03
收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
主题: Re: [edk2-devel] 回复: [edk2-devel] 回复: [PATCH v1 1/1] BaseTools: Align include guards policy

 

Hi Liming,
I agree this is not an urgent change, but will there be a timeframe when non-essential clean-up is going to happen?

Regards,
Pierre 




[-- Attachment #2: Type: text/html, Size: 4146 bytes --]

Re: [PATCH v1 1/1] BaseTools: Align include guards policy

[Sami Mujawar]

Hi Pierre,

Thank you for this patch.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar
-----Original Message-----
From: Pierre.Gondois@arm.com <Pierre.Gondois@arm.com> 
Sent: 25 January 2021 03:45 PM
To: devel@edk2.groups.io; bob.c.feng@intel.com; gaoliming@byosoft.com.cn
Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Subject: [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_CHECK_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

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

[Rebecca Cran]

On 1/25/21 8:45 AM, PierreGondois wrote:

> 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

> 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

I don't think this is in the coding standards, but newer copyright lines 
are generally placed above, not below, older ones.

-- 
Rebecca Cran