* [PATCH v1 1/1] BaseTools: Align include guards policy @ 2021-01-25 15:45 PierreGondois 2021-01-25 15:56 ` [edk2-devel] " PierreGondois ` (3 more replies) 0 siblings, 4 replies; 11+ messages in thread From: PierreGondois @ 2021-01-25 15:45 UTC (permalink / raw) To: devel, bob.c.feng, gaoliming; +Cc: sami.mujawar 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Align include guards policy 2021-01-25 15:45 [PATCH v1 1/1] BaseTools: Align include guards policy PierreGondois @ 2021-01-25 15:56 ` PierreGondois 2021-01-26 1:22 ` 回复: " gaoliming ` (2 subsequent siblings) 3 siblings, 0 replies; 11+ messages in thread From: PierreGondois @ 2021-01-25 15:56 UTC (permalink / raw) To: PierreGondois, devel [-- 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 --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* 回复: [PATCH v1 1/1] BaseTools: Align include guards policy 2021-01-25 15:45 [PATCH v1 1/1] BaseTools: Align include guards policy PierreGondois 2021-01-25 15:56 ` [edk2-devel] " PierreGondois @ 2021-01-26 1:22 ` gaoliming 2021-01-26 11:07 ` [edk2-devel] " Leif Lindholm 2021-02-04 10:22 ` Sami Mujawar 2021-02-15 22:08 ` [edk2-devel] " Rebecca Cran 3 siblings, 1 reply; 11+ messages in thread From: gaoliming @ 2021-01-26 1:22 UTC (permalink / raw) To: Pierre.Gondois, devel, bob.c.feng Cc: sami.mujawar, leif, 'Laszlo Ersek', 'Michael D Kinney', 'Andrew Fish' 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] 回复: [PATCH v1 1/1] BaseTools: Align include guards policy 2021-01-26 1:22 ` 回复: " gaoliming @ 2021-01-26 11:07 ` Leif Lindholm 2021-02-03 22:31 ` PierreGondois 0 siblings, 1 reply; 11+ messages in thread From: Leif Lindholm @ 2021-01-26 11:07 UTC (permalink / raw) To: devel, gaoliming Cc: Pierre.Gondois, bob.c.feng, sami.mujawar, 'Laszlo Ersek', 'Michael D Kinney', 'Andrew Fish' 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 > > > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] 回复: [PATCH v1 1/1] BaseTools: Align include guards policy 2021-01-26 11:07 ` [edk2-devel] " Leif Lindholm @ 2021-02-03 22:31 ` PierreGondois 2021-02-04 7:27 ` 回复: " gaoliming 0 siblings, 1 reply; 11+ messages in thread From: PierreGondois @ 2021-02-03 22:31 UTC (permalink / raw) To: Leif Lindholm, devel@edk2.groups.io, gaoliming@byosoft.com.cn Cc: bob.c.feng@intel.com, Sami Mujawar, 'Laszlo Ersek', 'Michael D Kinney', 'Andrew Fish' 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* 回复: [edk2-devel] 回复: [PATCH v1 1/1] BaseTools: Align include guards policy 2021-02-03 22:31 ` PierreGondois @ 2021-02-04 7:27 ` gaoliming 2021-02-04 10:16 ` Sami Mujawar 2021-02-15 22:03 ` [edk2-devel] 回复: " PierreGondois 0 siblings, 2 replies; 11+ messages in thread From: gaoliming @ 2021-02-04 7:27 UTC (permalink / raw) To: 'Pierre Gondois', 'Leif Lindholm', devel Cc: bob.c.feng, 'Sami Mujawar', 'Laszlo Ersek', 'Michael D Kinney', 'Andrew Fish' 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] 回复: [PATCH v1 1/1] BaseTools: Align include guards policy 2021-02-04 7:27 ` 回复: " gaoliming @ 2021-02-04 10:16 ` Sami Mujawar 2021-02-15 22:03 ` [edk2-devel] 回复: " PierreGondois 1 sibling, 0 replies; 11+ messages in thread From: Sami Mujawar @ 2021-02-04 10:16 UTC (permalink / raw) To: gaoliming, Pierre Gondois, 'Leif Lindholm', devel@edk2.groups.io Cc: bob.c.feng@intel.com, 'Laszlo Ersek', 'Michael D Kinney', 'Andrew Fish', nd 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] 回复: [edk2-devel] 回复: [PATCH v1 1/1] BaseTools: Align include guards policy 2021-02-04 7:27 ` 回复: " gaoliming 2021-02-04 10:16 ` Sami Mujawar @ 2021-02-15 22:03 ` PierreGondois 2021-02-18 5:42 ` 回复: " gaoliming 1 sibling, 1 reply; 11+ messages in thread From: PierreGondois @ 2021-02-15 22:03 UTC (permalink / raw) To: gaoliming, devel [-- 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 --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* 回复: [edk2-devel] 回复: [edk2-devel] 回复: [PATCH v1 1/1] BaseTools: Align include guards policy 2021-02-15 22:03 ` [edk2-devel] 回复: " PierreGondois @ 2021-02-18 5:42 ` gaoliming 0 siblings, 0 replies; 11+ messages in thread From: gaoliming @ 2021-02-18 5:42 UTC (permalink / raw) To: devel, pierre.gondois [-- 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 --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/1] BaseTools: Align include guards policy 2021-01-25 15:45 [PATCH v1 1/1] BaseTools: Align include guards policy PierreGondois 2021-01-25 15:56 ` [edk2-devel] " PierreGondois 2021-01-26 1:22 ` 回复: " gaoliming @ 2021-02-04 10:22 ` Sami Mujawar 2021-02-15 22:08 ` [edk2-devel] " Rebecca Cran 3 siblings, 0 replies; 11+ messages in thread From: Sami Mujawar @ 2021-02-04 10:22 UTC (permalink / raw) To: Pierre Gondois, devel@edk2.groups.io, bob.c.feng@intel.com, gaoliming@byosoft.com.cn Cc: nd 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Align include guards policy 2021-01-25 15:45 [PATCH v1 1/1] BaseTools: Align include guards policy PierreGondois ` (2 preceding siblings ...) 2021-02-04 10:22 ` Sami Mujawar @ 2021-02-15 22:08 ` Rebecca Cran 3 siblings, 0 replies; 11+ messages in thread From: Rebecca Cran @ 2021-02-15 22:08 UTC (permalink / raw) To: devel, pierre.gondois, bob.c.feng, gaoliming; +Cc: sami.mujawar 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-02-18 5:43 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-01-25 15:45 [PATCH v1 1/1] BaseTools: Align include guards policy PierreGondois 2021-01-25 15:56 ` [edk2-devel] " PierreGondois 2021-01-26 1:22 ` 回复: " gaoliming 2021-01-26 11:07 ` [edk2-devel] " Leif Lindholm 2021-02-03 22:31 ` PierreGondois 2021-02-04 7:27 ` 回复: " gaoliming 2021-02-04 10:16 ` Sami Mujawar 2021-02-15 22:03 ` [edk2-devel] 回复: " PierreGondois 2021-02-18 5:42 ` 回复: " gaoliming 2021-02-04 10:22 ` Sami Mujawar 2021-02-15 22:08 ` [edk2-devel] " Rebecca Cran
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox