[PATCH v1 1/1] BaseTools/Ecc: Make Ecc only check first include guard

[PATCH v1 1/1] BaseTools/Ecc: Make Ecc only check first include guard

[PierreGondois]

From: Pierre Gondois <Pierre.Gondois@arm.com>

The Ecc tool checks the format of the include guard. This check is
currently done on all the names following the '#ifndef' statement.
It should only be done on the first include guard.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3252
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
The changes can be seen at: https://github.com/PierreARM/edk2/tree/1640_Ecc_tool_corrections

 BaseTools/Source/Python/Ecc/Check.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/Python/Ecc/Check.py b/BaseTools/Source/Python/Ecc/Check.py
index 7a012617fd35..d82b42de0119 100644
--- a/BaseTools/Source/Python/Ecc/Check.py
+++ b/BaseTools/Source/Python/Ecc/Check.py
@@ -3,6 +3,7 @@
 #
 # Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
 # 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
@@ -1437,11 +1438,13 @@ class Check(object):

             SqlCommand = """select ID, Value from %s where Model = %s""" % (FileTable, MODEL_IDENTIFIER_MACRO_IFNDEF)
             RecordSet = EccGlobalData.gDb.TblFile.Exec(SqlCommand)
-            for Record in RecordSet:
-                Name = Record[1].replace('#ifndef', '').strip()
+            if RecordSet:
+                # Only check the first ifndef statement of the file
+                FirstDefine = sorted(RecordSet, key=lambda Record: Record[0])[0]
+                Name = FirstDefine[1].replace('#ifndef', '').strip()
                 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])
+                        EccGlobalData.gDb.TblReport.Insert(ERROR_NAMING_CONVENTION_CHECK_IFNDEF_STATEMENT, OtherMsg="The #ifndef name [%s] does not follow the rules" % (Name), BelongsToTable=FileTable, BelongsToItem=FirstDefine[0])

     # Rule for path name, variable name and function name
     # 1. First character should be upper case
--
2.17.1

回复: [edk2-devel] [PATCH v1 1/1] BaseTools/Ecc: Make Ecc only check first include guard

[gaoliming]

Is this checker for .h file only?

> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表
> PierreGondois
> 发送时间: 2021年3月10日 0:17
> 收件人: devel@edk2.groups.io; bob.c.feng@intel.com;
> gaoliming@byosoft.com.cn; yuwei.chen@intel.com
> 主题: [edk2-devel] [PATCH v1 1/1] BaseTools/Ecc: Make Ecc only check first
> include guard
> 
> From: Pierre Gondois <Pierre.Gondois@arm.com>
> 
> The Ecc tool checks the format of the include guard. This check is
> currently done on all the names following the '#ifndef' statement.
> It should only be done on the first include guard.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3252
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
> The changes can be seen at:
> https://github.com/PierreARM/edk2/tree/1640_Ecc_tool_corrections
> 
>  BaseTools/Source/Python/Ecc/Check.py | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/Ecc/Check.py
> b/BaseTools/Source/Python/Ecc/Check.py
> index 7a012617fd35..d82b42de0119 100644
> --- a/BaseTools/Source/Python/Ecc/Check.py
> +++ b/BaseTools/Source/Python/Ecc/Check.py
> @@ -3,6 +3,7 @@
>  #
>  # Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
>  # 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
> @@ -1437,11 +1438,13 @@ class Check(object):
> 
>              SqlCommand = """select ID, Value from %s where Model
> = %s""" % (FileTable, MODEL_IDENTIFIER_MACRO_IFNDEF)
>              RecordSet = EccGlobalData.gDb.TblFile.Exec(SqlCommand)
> -            for Record in RecordSet:
> -                Name = Record[1].replace('#ifndef', '').strip()
> +            if RecordSet:
> +                # Only check the first ifndef statement of the file
> +                FirstDefine = sorted(RecordSet, key=lambda Record:
> Record[0])[0]
> +                Name = FirstDefine[1].replace('#ifndef', '').strip()
>                  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])
> +
> EccGlobalData.gDb.TblReport.Insert(ERROR_NAMING_CONVENTION_CHECK
> _IFNDEF_STATEMENT, OtherMsg="The #ifndef name [%s] does not follow the
> rules" % (Name), BelongsToTable=FileTable, BelongsToItem=FirstDefine[0])
> 
>      # Rule for path name, variable name and function name
>      # 1. First character should be upper case
> --
> 2.17.1
> 
> 
> 
> 
> 

Re: [edk2-devel] 回复: [edk2-devel] [PATCH v1 1/1] BaseTools/Ecc: Make Ecc only check first include guard

[PierreGondois]

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

Hello Liming,
Yes this check is only done for '.h' files. Cf the bit of code from the same file:

if os.path.splitext(F)[1] in ('.h'):
self.NamingConventionCheckIfndefStatement(FileTable)

I think this is normal, the format of the initial 'ifndef' needs to be formatted as 'Name_', but not necessarilly all the other 'ifndef' in the file.
I added the arm copyright two times, I will submit a v2 if the patch goes forward.

Regards,
Pierre

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

回复: [edk2-devel] 回复: [edk2-devel] [PATCH v1 1/1] BaseTools/Ecc: Make Ecc only check first include guard

[gaoliming]

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

Pierre:

 I see. This change is good to me. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

 

Thanks

Liming

发件人: PierreGondois <pierre.gondois@arm.com> 
发送时间: 2021年3月11日 17:26
收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
主题: Re: [edk2-devel] 回复: [edk2-devel] [PATCH v1 1/1] BaseTools/Ecc: Make Ecc only check first include guard

 

Hello Liming,
Yes this check is only done for '.h' files. Cf the bit of code from the same file:

if os.path.splitext(F)[1] in ('.h'):
  self.NamingConventionCheckIfndefStatement(FileTable)

I think this is normal, the format of the initial 'ifndef' needs to be formatted as 'Name_', but not necessarilly all the other 'ifndef' in the file.
I added the arm copyright two times, I will submit a v2 if the patch goes forward.

Regards,
Pierre 


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

Re: [edk2-devel] [PATCH v1 1/1] BaseTools/Ecc: Make Ecc only check first include guard

[Bob Feng]

This patch looks good to me.
After you remove the duplicated arm copyright, Reviewed-by: Bob Feng <bob.c.feng@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of PierreGondois
Sent: Wednesday, March 10, 2021 12:17 AM
To: devel@edk2.groups.io; Feng, Bob C <bob.c.feng@intel.com>; gaoliming@byosoft.com.cn; Chen, Christine <yuwei.chen@intel.com>
Subject: [edk2-devel] [PATCH v1 1/1] BaseTools/Ecc: Make Ecc only check first include guard

From: Pierre Gondois <Pierre.Gondois@arm.com>

The Ecc tool checks the format of the include guard. This check is currently done on all the names following the '#ifndef' statement.
It should only be done on the first include guard.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3252
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
The changes can be seen at: https://github.com/PierreARM/edk2/tree/1640_Ecc_tool_corrections

 BaseTools/Source/Python/Ecc/Check.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/Python/Ecc/Check.py b/BaseTools/Source/Python/Ecc/Check.py
index 7a012617fd35..d82b42de0119 100644
--- a/BaseTools/Source/Python/Ecc/Check.py
+++ b/BaseTools/Source/Python/Ecc/Check.py
@@ -3,6 +3,7 @@
 #
 # Copyright (c) 2021, Arm Limited. All rights reserved.<BR>  # 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 @@ -1437,11 +1438,13 @@ class Check(object):

             SqlCommand = """select ID, Value from %s where Model = %s""" % (FileTable, MODEL_IDENTIFIER_MACRO_IFNDEF)
             RecordSet = EccGlobalData.gDb.TblFile.Exec(SqlCommand)
-            for Record in RecordSet:
-                Name = Record[1].replace('#ifndef', '').strip()
+            if RecordSet:
+                # Only check the first ifndef statement of the file
+                FirstDefine = sorted(RecordSet, key=lambda Record: Record[0])[0]
+                Name = FirstDefine[1].replace('#ifndef', '').strip()
                 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])
+
+ EccGlobalData.gDb.TblReport.Insert(ERROR_NAMING_CONVENTION_CHECK_IFNDE
+ F_STATEMENT, OtherMsg="The #ifndef name [%s] does not follow the 
+ rules" % (Name), BelongsToTable=FileTable, 
+ BelongsToItem=FirstDefine[0])

     # Rule for path name, variable name and function name
     # 1. First character should be upper case
--
2.17.1