From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web12.4576.1612423676212935838 for ; Wed, 03 Feb 2021 23:27:58 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.242, mailfrom: gaoliming@byosoft.com.cn) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Thu, 04 Feb 2021 15:27:36 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: "'Pierre Gondois'" , "'Leif Lindholm'" , Cc: , "'Sami Mujawar'" , "'Laszlo Ersek'" , "'Michael D Kinney'" , "'Andrew Fish'" References: <20210125154525.18449-1-Pierre.Gondois@arm.com> <00f201d6f381$a8dd8970$fa989c50$@byosoft.com.cn>,<20210126110744.GM1664@vanye> In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0g5Zue5aSNOiBbUEFUQ0ggdjEgMS8xXSBCYXNlVG9vbHM6IEFsaWduIGluY2x1ZGUgZ3VhcmRzIHBvbGljeQ==?= Date: Thu, 4 Feb 2021 15:27:44 +0800 Message-ID: <006501d6fac7$3b08d9c0$b11a8d40$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQGlA7GGrJ2S6a9oLVPP+lkyZnS4mAHFjykCAxicQDsCg2tlfKpwd75A Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Pierre: I get Leif point. I also see clang warning https://clang.llvm.org/docs/D= iagnosticsReference.html#wreserved-id-macro I agree the compiler may reserve the macro starting with _ prefix. But s= o far, I don't get any error report that the macro conflict with the compil= er reserved macros. So, I don't think this change is urgent.=20 I would like to agree to add this ECC checker. But, the clean up to the = existing coding can be planned later. Thanks Liming > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: Pierre Gondois > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2021=E5=B9=B42=E6=9C=884=E6=97=A5 = 6:32 > =E6=94=B6=E4=BB=B6=E4=BA=BA: Leif Lindholm ; devel@ed= k2.groups.io; > gaoliming@byosoft.com.cn > =E6=8A=84=E9=80=81: bob.c.feng@intel.com; Sami Mujawar ; > 'Laszlo Ersek' ; 'Michael D Kinney' > ; 'Andrew Fish' > =E4=B8=BB=E9=A2=98: Re: [edk2-devel] =E5=9B=9E=E5=A4=8D: [PATCH v1 1/1] = BaseTools: Align include guards > policy >=20 > 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 >=20 > Regards, > Pierre >=20 >=20 > From: Leif Lindholm > Sent: Tuesday, January 26, 2021 11:07 AM > To: devel@edk2.groups.io ; > gaoliming@byosoft.com.cn > Cc: Pierre Gondois ; bob.c.feng@intel.com > ; Sami Mujawar ; 'Laszlo > Ersek' ; 'Michael D Kinney' > ; 'Andrew Fish' > Subject: Re: [edk2-devel] =E5=9B=9E=E5=A4=8D: [PATCH v1 1/1] BaseTools: = Align include > guards policy >=20 > Hi Liming, >=20 > 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. >=20 > Macros starting with leading _ are reserved for toolchain use. > Some toolchains, i.e. clang, have dedicated warnings for this. >=20 > 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. >=20 > Either way, this patch sounds like a useful change. > Adding the check for the end of the string would also help improving > code consistency. >=20 > / > Leif >=20 > 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 styl= e > > __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 > > > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > > > =E5=8F=91=E4=BB=B6=E4=BA=BA: Pierre.Gondois@arm.com > > > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2021=E5=B9=B41=E6=9C=8825=E6= =97=A5 23:45 > > > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io; bob.c.feng@intel.= com; > > > gaoliming@byosoft.com.cn > > > =E6=8A=84=E9=80=81: sami.mujawar@arm.com > > > =E4=B8=BB=E9=A2=98: [PATCH v1 1/1] BaseTools: Align include guards p= olicy > > > > > > From: Pierre Gondois > > > > > > 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 > > > --- > > > 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= .
> > > +# Copyright (c) 2021, Arm Limited. All rights reserved.
> > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > > # > > > from __future__ import absolute_import > > > @@ -1438,7 +1439,7 @@ class Check(object): > > > RecordSet =3D > EccGlobalData.gDb.TblFile.Exec(SqlCommand) > > > for Record in RecordSet: > > > Name =3D Record[1].replace('#ifndef', '').strip() > > > - if Name[-1] !=3D '_': > > > + if Name[0] =3D=3D '_' or Name[-1] !=3D '_' or Name[= -2] =3D=3D > '_': > > > if not > > > > EccGlobalData.gException.IsException(ERROR_NAMING_CONVENTION_CHE > > > CK_IFNDEF_STATEMENT, Name): > > > > > > > EccGlobalData.gDb.TblReport.Insert(ERROR_NAMING_CONVENTION_CHECK > > > _IFNDEF_STATEMENT, OtherMsg=3D"The #ifndef name [%s] does not follow > the > > > rules" % (Name), BelongsToTable=3DFileTable, BelongsToItem=3DRecord[= 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= .
> > > +# Copyright (c) 2021, Arm Limited. All rights reserved.
> > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > > # > > > > > > @@ -161,7 +162,7 @@ gEccErrorMessage =3D { > > > 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 p= ostfix > > > 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 shoul= d be > > upper > > > case 2. Must contain lower case characters 3. No white space charact= ers > 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 shoul= d be > > upper > > > case 2. Must contain lower case characters 3. No white space > > characters""", > > > -- > > > 2.17.1 > > > > > > > > > > > >=20 > > > > > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended rec= ipient, > 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 i= n any > medium. Thank you.