[PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC

[PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC

[Chang, Abner]

From: Abner Chang <abner.chang@amd.com>

BZ #1766

Remove the entire 5.4.2.2 section.
We are not allowed to use upper-case STATIC in the source file now.
Just follow C standard and use the lower-case 'static'.

Leave the macro "#deifne STATIC static" there without removing
it to keep the backward compatable.

Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
---
 5_source_files/54_code_file_structure.md | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/5_source_files/54_code_file_structure.md b/5_source_files/54_code_file_structure.md
index 0c4d6a2..9acc620 100644
--- a/5_source_files/54_code_file_structure.md
+++ b/5_source_files/54_code_file_structure.md
@@ -267,19 +267,3 @@ specified in Section 5.4.1.3 "Compile-Time Names".
 Thus, while it might be legal C, do **not** declare external variables anywhere
 other than at the top level of a file as specified by this document.
 
-#### 5.4.2.2 Static
-
-An object declared `STATIC` has either file or block scope.
-
-##### 5.4.2.2.1 Do not reuse an object or function identifier with static storage duration.
-
-Throughout the set of source files defined within a single .inf file, do not
-reuse an identifier with static storage duration. The compiler may not be
-confused by this, but the user may confuse unrelated variables with the same
-name.
-
-##### 5.4.2.2.2 Functions should not be declared STATIC.
-
-Some source-level debuggers are unable to resolve static functions. Until it
-can be verified that no one is dependent upon a debugger with this limitation,
-it is strongly recommended that functions not be declared static.
-- 
2.37.1.windows.1

Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC

[Ni, Ray]

Abner,
>From what I read, the idea of BZ1766 is to add recommendations to use static for local symbols.

"Add recommendations to the EDK II C Coding Standards Specification to use
'static' for all functions and global variables that are not referenced
outside the current C file."

Do you want to capture that in the EDKII C Coding Standard?

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang,
> Abner via groups.io
> Sent: Tuesday, November 22, 2022 12:47 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section
> 5.4.2.2 STATIC
> 
> From: Abner Chang <abner.chang@amd.com>
> 
> BZ #1766
> 
> Remove the entire 5.4.2.2 section.
> We are not allowed to use upper-case STATIC in the source file now.
> Just follow C standard and use the lower-case 'static'.
> 
> Leave the macro "#deifne STATIC static" there without removing
> it to keep the backward compatable.
> 
> Signed-off-by: Abner Chang <abner.chang@amd.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  5_source_files/54_code_file_structure.md | 16 ----------------
>  1 file changed, 16 deletions(-)
> 
> diff --git a/5_source_files/54_code_file_structure.md
> b/5_source_files/54_code_file_structure.md
> index 0c4d6a2..9acc620 100644
> --- a/5_source_files/54_code_file_structure.md
> +++ b/5_source_files/54_code_file_structure.md
> @@ -267,19 +267,3 @@ specified in Section 5.4.1.3 "Compile-Time Names".
>  Thus, while it might be legal C, do **not** declare external variables
> anywhere
>  other than at the top level of a file as specified by this document.
> 
> -#### 5.4.2.2 Static
> -
> -An object declared `STATIC` has either file or block scope.
> -
> -##### 5.4.2.2.1 Do not reuse an object or function identifier with static
> storage duration.
> -
> -Throughout the set of source files defined within a single .inf file, do not
> -reuse an identifier with static storage duration. The compiler may not be
> -confused by this, but the user may confuse unrelated variables with the
> same
> -name.
> -
> -##### 5.4.2.2.2 Functions should not be declared STATIC.
> -
> -Some source-level debuggers are unable to resolve static functions. Until it
> -can be verified that no one is dependent upon a debugger with this
> limitation,
> -it is strongly recommended that functions not be declared static.
> --
> 2.37.1.windows.1
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC

[Chang, Abner]

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

[AMD Official Use Only - General]

Hi Ray,
>From the last week edk2 Bug triage meeting, my understanding from Mike was to remove the entire 5.4.2.2 section and no need to add anything because we already mention at the beginning in CCS to follow C dialect.

@Kinney, Michael D and @Liming Gao, is that correct?
Abner

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Tuesday, November 22, 2022 1:48 PM
> To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section
> 5.4.2.2 STATIC
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> Abner,
> From what I read, the idea of BZ1766 is to add recommendations to use static
> for local symbols.
> 
> "Add recommendations to the EDK II C Coding Standards Specification to use
> 'static' for all functions and global variables that are not referenced outside
> the current C file."
> 
> Do you want to capture that in the EDKII C Coding Standard?
> 
> Thanks,
> Ray
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang,
> > Abner via groups.io
> > Sent: Tuesday, November 22, 2022 12:47 PM
> > To: devel@edk2.groups.io
> > Cc: Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Subject: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section
> > 5.4.2.2 STATIC
> >
> > From: Abner Chang <abner.chang@amd.com>
> >
> > BZ #1766
> >
> > Remove the entire 5.4.2.2 section.
> > We are not allowed to use upper-case STATIC in the source file now.
> > Just follow C standard and use the lower-case 'static'.
> >
> > Leave the macro "#deifne STATIC static" there without removing it to
> > keep the backward compatable.
> >
> > Signed-off-by: Abner Chang <abner.chang@amd.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > ---
> >  5_source_files/54_code_file_structure.md | 16 ----------------
> >  1 file changed, 16 deletions(-)
> >
> > diff --git a/5_source_files/54_code_file_structure.md
> > b/5_source_files/54_code_file_structure.md
> > index 0c4d6a2..9acc620 100644
> > --- a/5_source_files/54_code_file_structure.md
> > +++ b/5_source_files/54_code_file_structure.md
> > @@ -267,19 +267,3 @@ specified in Section 5.4.1.3 "Compile-Time Names".
> >  Thus, while it might be legal C, do **not** declare external
> > variables anywhere  other than at the top level of a file as specified
> > by this document.
> >
> > -#### 5.4.2.2 Static
> > -
> > -An object declared `STATIC` has either file or block scope.
> > -
> > -##### 5.4.2.2.1 Do not reuse an object or function identifier with
> > static storage duration.
> > -
> > -Throughout the set of source files defined within a single .inf file,
> > do not -reuse an identifier with static storage duration. The compiler
> > may not be -confused by this, but the user may confuse unrelated
> > variables with the same -name.
> > -
> > -##### 5.4.2.2.2 Functions should not be declared STATIC.
> > -
> > -Some source-level debuggers are unable to resolve static functions.
> > Until it -can be verified that no one is dependent upon a debugger
> > with this limitation, -it is strongly recommended that functions not
> > be declared static.
> > --
> > 2.37.1.windows.1
> >
> >
> >
> > 
> >

[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 21209 bytes --]

Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC

[Michael D Kinney]

Hi Abner,

Removing that section 5.4.2.2 is required to close this bug.

Meaning of 'static' is covered by the ANSI C standards.

Use of 'static' for non-public variable/functions in EDK II
libraries/modules is recommended.

However, it is not required.  It is recommended to reduce chances
of symbol conflicts at link time.  Current approach is if a link
failure occurs for multiply defined symbols and those are non-public
symbols, the 'static' attribute can be applied to the non-public
symbols in the components that generated the link failure.

It may be good to mention this recommendation in the CSS.

I will let you decide when this recommendation needs to be
added to CSS.

Mike

> -----Original Message-----
> From: Chang, Abner <Abner.Chang@amd.com>
> Sent: Monday, November 21, 2022 10:08 PM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>
> Subject: RE: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC
> 
> [AMD Official Use Only - General]
> 
> Hi Ray,
> From the last week edk2 Bug triage meeting, my understanding from Mike was to remove the entire 5.4.2.2 section and no need to
> add anything because we already mention at the beginning in CCS to follow C dialect.
> 
> @Kinney, Michael D and @Liming Gao, is that correct?
> Abner
> 
> > -----Original Message-----
> > From: Ni, Ray <ray.ni@intel.com>
> > Sent: Tuesday, November 22, 2022 1:48 PM
> > To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: RE: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section
> > 5.4.2.2 STATIC
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > Abner,
> > From what I read, the idea of BZ1766 is to add recommendations to use static
> > for local symbols.
> >
> > "Add recommendations to the EDK II C Coding Standards Specification to use
> > 'static' for all functions and global variables that are not referenced outside
> > the current C file."
> >
> > Do you want to capture that in the EDKII C Coding Standard?
> >
> > Thanks,
> > Ray
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang,
> > > Abner via groups.io
> > > Sent: Tuesday, November 22, 2022 12:47 PM
> > > To: devel@edk2.groups.io
> > > Cc: Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> > > <michael.d.kinney@intel.com>
> > > Subject: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section
> > > 5.4.2.2 STATIC
> > >
> > > From: Abner Chang <abner.chang@amd.com>
> > >
> > > BZ #1766
> > >
> > > Remove the entire 5.4.2.2 section.
> > > We are not allowed to use upper-case STATIC in the source file now.
> > > Just follow C standard and use the lower-case 'static'.
> > >
> > > Leave the macro "#deifne STATIC static" there without removing it to
> > > keep the backward compatable.
> > >
> > > Signed-off-by: Abner Chang <abner.chang@amd.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > ---
> > >  5_source_files/54_code_file_structure.md | 16 ----------------
> > >  1 file changed, 16 deletions(-)
> > >
> > > diff --git a/5_source_files/54_code_file_structure.md
> > > b/5_source_files/54_code_file_structure.md
> > > index 0c4d6a2..9acc620 100644
> > > --- a/5_source_files/54_code_file_structure.md
> > > +++ b/5_source_files/54_code_file_structure.md
> > > @@ -267,19 +267,3 @@ specified in Section 5.4.1.3 "Compile-Time Names".
> > >  Thus, while it might be legal C, do **not** declare external
> > > variables anywhere  other than at the top level of a file as specified
> > > by this document.
> > >
> > > -#### 5.4.2.2 Static
> > > -
> > > -An object declared `STATIC` has either file or block scope.
> > > -
> > > -##### 5.4.2.2.1 Do not reuse an object or function identifier with
> > > static storage duration.
> > > -
> > > -Throughout the set of source files defined within a single .inf file,
> > > do not -reuse an identifier with static storage duration. The compiler
> > > may not be -confused by this, but the user may confuse unrelated
> > > variables with the same -name.
> > > -
> > > -##### 5.4.2.2.2 Functions should not be declared STATIC.
> > > -
> > > -Some source-level debuggers are unable to resolve static functions.
> > > Until it -can be verified that no one is dependent upon a debugger
> > > with this limitation, -it is strongly recommended that functions not
> > > be declared static.
> > > --
> > > 2.37.1.windows.1
> > >
> > >
> > >
> > > 
> > >

Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC

[Pedro Falcato]

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

On Tue, Nov 22, 2022 at 6:10 PM Michael D Kinney <michael.d.kinney@intel.com>
wrote:

> Hi Abner,
>
> Removing that section 5.4.2.2 is required to close this bug.
>
> Meaning of 'static' is covered by the ANSI C standards.
>

Mike,

Sorry for spinning off a bit, but if we're dropping STATIC, can we also
drop the other defines (like CONST, VOID, etc)?
Potentially culminating into adopting C99 (does any EDK2-relevant C
compiler not support it?).

Pedro

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

Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC

[Michael D Kinney]

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

Hi Pedro,

CONST and VOID are defined in Section 2.3 of the UEFI Specification.
So we need to keep them to consume .h files based on UEF Spec contents.

STATIC is not part of any industry standard spec.

Mike


From: Pedro Falcato <pedro.falcato@gmail.com>
Sent: Tuesday, November 22, 2022 2:12 PM
To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Chang, Abner <Abner.Chang@amd.com>; Ni, Ray <ray.ni@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC

On Tue, Nov 22, 2022 at 6:10 PM Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:
Hi Abner,

Removing that section 5.4.2.2 is required to close this bug.

Meaning of 'static' is covered by the ANSI C standards.

Mike,

Sorry for spinning off a bit, but if we're dropping STATIC, can we also drop the other defines (like CONST, VOID, etc)?
Potentially culminating into adopting C99 (does any EDK2-relevant C compiler not support it?).

Pedro

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

Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC

[Chang, Abner]

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

[AMD Official Use Only - General]

Hi Ray and Mike,
Besides removing 5.4.2.2.1 and 5.4.2.2.2,  the "static" should be also removed from 5.6.1.2 section title.  I just went back to check the comment of BZ1766 and yes we can add the recommendation of using static.  Just having it in 5.4.2.2. 
Will send the patch update.
Thanks
Abner

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> D Kinney via groups.io
> Sent: Wednesday, November 23, 2022 12:32 AM
> To: Chang, Abner <Abner.Chang@amd.com>; Ni, Ray <ray.ni@intel.com>;
> devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section
> 5.4.2.2 STATIC
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> Hi Abner,
> 
> Removing that section 5.4.2.2 is required to close this bug.
> 
> Meaning of 'static' is covered by the ANSI C standards.
> 
> Use of 'static' for non-public variable/functions in EDK II libraries/modules is
> recommended.
> 
> However, it is not required.  It is recommended to reduce chances of symbol
> conflicts at link time.  Current approach is if a link failure occurs for multiply
> defined symbols and those are non-public symbols, the 'static' attribute can
> be applied to the non-public symbols in the components that generated the
> link failure.
> 
> It may be good to mention this recommendation in the CSS.
> 
> I will let you decide when this recommendation needs to be added to CSS.
> 
> Mike
> 
> > -----Original Message-----
> > From: Chang, Abner <Abner.Chang@amd.com>
> > Sent: Monday, November 21, 2022 10:08 PM
> > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Kinney, Michael
> > D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>
> > Subject: RE: [edk2-devel] [PATCH] edk II C Coding Standard: Remove
> > section 5.4.2.2 STATIC
> >
> > [AMD Official Use Only - General]
> >
> > Hi Ray,
> > From the last week edk2 Bug triage meeting, my understanding from Mike
> > was to remove the entire 5.4.2.2 section and no need to add anything
> because we already mention at the beginning in CCS to follow C dialect.
> >
> > @Kinney, Michael D and @Liming Gao, is that correct?
> > Abner
> >
> > > -----Original Message-----
> > > From: Ni, Ray <ray.ni@intel.com>
> > > Sent: Tuesday, November 22, 2022 1:48 PM
> > > To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Subject: RE: [edk2-devel] [PATCH] edk II C Coding Standard: Remove
> > > section
> > > 5.4.2.2 STATIC
> > >
> > > Caution: This message originated from an External Source. Use proper
> > > caution when opening attachments, clicking links, or responding.
> > >
> > >
> > > Abner,
> > > From what I read, the idea of BZ1766 is to add recommendations to
> > > use static for local symbols.
> > >
> > > "Add recommendations to the EDK II C Coding Standards Specification
> > > to use 'static' for all functions and global variables that are not
> > > referenced outside the current C file."
> > >
> > > Do you want to capture that in the EDKII C Coding Standard?
> > >
> > > Thanks,
> > > Ray
> > >
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > Chang, Abner via groups.io
> > > > Sent: Tuesday, November 22, 2022 12:47 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> > > > <michael.d.kinney@intel.com>
> > > > Subject: [edk2-devel] [PATCH] edk II C Coding Standard: Remove
> > > > section
> > > > 5.4.2.2 STATIC
> > > >
> > > > From: Abner Chang <abner.chang@amd.com>
> > > >
> > > > BZ #1766
> > > >
> > > > Remove the entire 5.4.2.2 section.
> > > > We are not allowed to use upper-case STATIC in the source file now.
> > > > Just follow C standard and use the lower-case 'static'.
> > > >
> > > > Leave the macro "#deifne STATIC static" there without removing it
> > > > to keep the backward compatable.
> > > >
> > > > Signed-off-by: Abner Chang <abner.chang@amd.com>
> > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > > ---
> > > >  5_source_files/54_code_file_structure.md | 16 ----------------
> > > >  1 file changed, 16 deletions(-)
> > > >
> > > > diff --git a/5_source_files/54_code_file_structure.md
> > > > b/5_source_files/54_code_file_structure.md
> > > > index 0c4d6a2..9acc620 100644
> > > > --- a/5_source_files/54_code_file_structure.md
> > > > +++ b/5_source_files/54_code_file_structure.md
> > > > @@ -267,19 +267,3 @@ specified in Section 5.4.1.3 "Compile-Time
> Names".
> > > >  Thus, while it might be legal C, do **not** declare external
> > > > variables anywhere  other than at the top level of a file as
> > > > specified by this document.
> > > >
> > > > -#### 5.4.2.2 Static
> > > > -
> > > > -An object declared `STATIC` has either file or block scope.
> > > > -
> > > > -##### 5.4.2.2.1 Do not reuse an object or function identifier
> > > > with static storage duration.
> > > > -
> > > > -Throughout the set of source files defined within a single .inf
> > > > file, do not -reuse an identifier with static storage duration.
> > > > The compiler may not be -confused by this, but the user may
> > > > confuse unrelated variables with the same -name.
> > > > -
> > > > -##### 5.4.2.2.2 Functions should not be declared STATIC.
> > > > -
> > > > -Some source-level debuggers are unable to resolve static functions.
> > > > Until it -can be verified that no one is dependent upon a debugger
> > > > with this limitation, -it is strongly recommended that functions
> > > > not be declared static.
> > > > --
> > > > 2.37.1.windows.1
> > > >
> > > >
> > > >
> > > >
> > > >
> 
> 
> 
> 
> 

[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 20285 bytes --]

Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC

[Ard Biesheuvel]

On Tue, 22 Nov 2022 at 19:10, Michael D Kinney
<michael.d.kinney@intel.com> wrote:
>
> Hi Abner,
>
> Removing that section 5.4.2.2 is required to close this bug.
>
> Meaning of 'static' is covered by the ANSI C standards.
>
> Use of 'static' for non-public variable/functions in EDK II
> libraries/modules is recommended.
>
> However, it is not required.  It is recommended to reduce chances
> of symbol conflicts at link time.  Current approach is if a link
> failure occurs for multiply defined symbols and those are non-public
> symbols, the 'static' attribute can be applied to the non-public
> symbols in the components that generated the link failure.
>
> It may be good to mention this recommendation in the CSS.
>
> I will let you decide when this recommendation needs to be
> added to CSS.
>

'static' is not just a tool to avoid symbol conflicts. It also avoids
abuse of symbols that are assumed to have a private nature but can be
linked to nonetheless (e.g., in static libraries). Ideally, any
library should only export the symbols that it defines as part of its
interface, although this is not currently feasible of a library
consists of multiple object files.

Another thing to keep in mind is that static is used by the compiler
to make inferences about the value. A static global variable can only
be modified by the code in the same compilation unit, and so the
compiler is free to optimize accesses or perform constant propagation
and drop it entirely if it only observes reads and no writes from the
variable.

I consider it good developer hygiene to always use static on global
symbols (code and data) unless the symbol needs to be shared with
other compilation units.

Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC

[Michael D Kinney]

Ard,

I agree it should be a strong recommendation for all of these reasons.

There is a patch review for this spec for use of 'static'.  Can you 
please provide feedback with your recommended content?

Thanks,

Mike

> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Monday, November 28, 2022 1:08 AM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Chang, Abner <Abner.Chang@amd.com>; Ni, Ray <ray.ni@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC
> 
> On Tue, 22 Nov 2022 at 19:10, Michael D Kinney
> <michael.d.kinney@intel.com> wrote:
> >
> > Hi Abner,
> >
> > Removing that section 5.4.2.2 is required to close this bug.
> >
> > Meaning of 'static' is covered by the ANSI C standards.
> >
> > Use of 'static' for non-public variable/functions in EDK II
> > libraries/modules is recommended.
> >
> > However, it is not required.  It is recommended to reduce chances
> > of symbol conflicts at link time.  Current approach is if a link
> > failure occurs for multiply defined symbols and those are non-public
> > symbols, the 'static' attribute can be applied to the non-public
> > symbols in the components that generated the link failure.
> >
> > It may be good to mention this recommendation in the CSS.
> >
> > I will let you decide when this recommendation needs to be
> > added to CSS.
> >
> 
> 'static' is not just a tool to avoid symbol conflicts. It also avoids
> abuse of symbols that are assumed to have a private nature but can be
> linked to nonetheless (e.g., in static libraries). Ideally, any
> library should only export the symbols that it defines as part of its
> interface, although this is not currently feasible of a library
> consists of multiple object files.
> 
> Another thing to keep in mind is that static is used by the compiler
> to make inferences about the value. A static global variable can only
> be modified by the code in the same compilation unit, and so the
> compiler is free to optimize accesses or perform constant propagation
> and drop it entirely if it only observes reads and no writes from the
> variable.
> 
> I consider it good developer hygiene to always use static on global
> symbols (code and data) unless the symbol needs to be shared with
> other compilation units.

Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC

[Chang, Abner]

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

[AMD Official Use Only - General]

HI Ard, attach the patch review for the 'static'.  Please check it and provide your feedback on it if you think the phrase is not strong enough.

Thanks
Abner

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Monday, November 28, 2022 11:41 PM
> To: Ard Biesheuvel <ardb@kernel.org>; devel@edk2.groups.io; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Cc: Chang, Abner <Abner.Chang@amd.com>; Ni, Ray <ray.ni@intel.com>;
> Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: RE: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section
> 5.4.2.2 STATIC
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> Ard,
> 
> I agree it should be a strong recommendation for all of these reasons.
> 
> There is a patch review for this spec for use of 'static'.  Can you please
> provide feedback with your recommended content?
> 
> Thanks,
> 
> Mike
> 
> > -----Original Message-----
> > From: Ard Biesheuvel <ardb@kernel.org>
> > Sent: Monday, November 28, 2022 1:08 AM
> > To: devel@edk2.groups.io; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Cc: Chang, Abner <Abner.Chang@amd.com>; Ni, Ray <ray.ni@intel.com>;
> > Gao, Liming <gaoliming@byosoft.com.cn>
> > Subject: Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove
> > section 5.4.2.2 STATIC
> >
> > On Tue, 22 Nov 2022 at 19:10, Michael D Kinney
> > <michael.d.kinney@intel.com> wrote:
> > >
> > > Hi Abner,
> > >
> > > Removing that section 5.4.2.2 is required to close this bug.
> > >
> > > Meaning of 'static' is covered by the ANSI C standards.
> > >
> > > Use of 'static' for non-public variable/functions in EDK II
> > > libraries/modules is recommended.
> > >
> > > However, it is not required.  It is recommended to reduce chances of
> > > symbol conflicts at link time.  Current approach is if a link
> > > failure occurs for multiply defined symbols and those are non-public
> > > symbols, the 'static' attribute can be applied to the non-public
> > > symbols in the components that generated the link failure.
> > >
> > > It may be good to mention this recommendation in the CSS.
> > >
> > > I will let you decide when this recommendation needs to be added to
> > > CSS.
> > >
> >
> > 'static' is not just a tool to avoid symbol conflicts. It also avoids
> > abuse of symbols that are assumed to have a private nature but can be
> > linked to nonetheless (e.g., in static libraries). Ideally, any
> > library should only export the symbols that it defines as part of its
> > interface, although this is not currently feasible of a library
> > consists of multiple object files.
> >
> > Another thing to keep in mind is that static is used by the compiler
> > to make inferences about the value. A static global variable can only
> > be modified by the code in the same compilation unit, and so the
> > compiler is free to optimize accesses or perform constant propagation
> > and drop it entirely if it only observes reads and no writes from the
> > variable.
> >
> > I consider it good developer hygiene to always use static on global
> > symbols (code and data) unless the symbol needs to be shared with
> > other compilation units.

[-- Attachment #2: Type: message/rfc822, Size: 14745 bytes --]

From: "Chang, Abner via groups.io" <abner.chang=amd.com@groups.io>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Ray Ni <ray.ni@intel.com>, Michael D Kinney <michael.d.kinney@intel.com>
Subject: [edk2-devel] [PATCH V2] edk II C Coding Standard: Remove section 5.4.2.2 STATIC
Date: Sat, 26 Nov 2022 09:02:57 +0000
Message-ID: <172B1708C307B48B.11309@groups.io>

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


From: Abner Chang <abner.chang@amd.com>

BZ #1766

Revise the sections that mention the usage of "static" and
remove section 5.4.2.2.1 and 5.4.2.2.2 section.
We are not allowed to use upper-case STATIC in the source
file now.
Just follow C standard and use the lower-case 'static'.

Leave the macro "#deifne STATIC static" there without
removing it to keep the backward compatible.

Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
---
 5_source_files/54_code_file_structure.md    | 19 ++++---------------
 5_source_files/56_declarations_and_types.md |  2 +-
 2 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/5_source_files/54_code_file_structure.md b/5_source_files/54_code_file_structure.md
index 0c4d6a2..6a9fcc7 100644
--- a/5_source_files/54_code_file_structure.md
+++ b/5_source_files/54_code_file_structure.md
@@ -267,19 +267,8 @@ specified in Section 5.4.1.3 "Compile-Time Names".
 Thus, while it might be legal C, do **not** declare external variables anywhere
 other than at the top level of a file as specified by this document.

-#### 5.4.2.2 Static
+#### 5.4.2.2 static

-An object declared `STATIC` has either file or block scope.
-
-##### 5.4.2.2.1 Do not reuse an object or function identifier with static storage duration.
-
-Throughout the set of source files defined within a single .inf file, do not
-reuse an identifier with static storage duration. The compiler may not be
-confused by this, but the user may confuse unrelated variables with the same
-name.
-
-##### 5.4.2.2.2 Functions should not be declared STATIC.
-
-Some source-level debuggers are unable to resolve static functions. Until it
-can be verified that no one is dependent upon a debugger with this limitation,
-it is strongly recommended that functions not be declared static.
+Use static for the variables and functions those are non-public to other source
+files is encouraged. This reduces the chances of symbol redefined error during
+link process.
\ No newline at end of file
diff --git a/5_source_files/56_declarations_and_types.md b/5_source_files/56_declarations_and_types.md
index ec1803d..db98b95 100644
--- a/5_source_files/56_declarations_and_types.md
+++ b/5_source_files/56_declarations_and_types.md
@@ -38,7 +38,7 @@
 Any abstract type that is defined must be constructed from other abstract types
 or from common EFI data types.

-#### 5.6.1.2 The use of int, unsigned, char, void, static, long is a violation of the coding convention.
+#### 5.6.1.2 The use of int, unsigned, char, void, long and the data types those are defined in EFI Data Types is a violation of the coding convention.

 The corresponding EFI types must be used instead.

--
2.37.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96631): https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F96631&amp;data=05%7C01%7Cabner.chang%40amd.com%7C4b16779158484971fb8308dacf8d1ce6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638050502240293052%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=eseelCW8m4aLLXn%2FkC14SPTMX4UA6B9g%2BI4Q5q2W1UU%3D&amp;reserved=0
Mute This Topic: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F95269399%2F7039027&amp;data=05%7C01%7Cabner.chang%40amd.com%7C4b16779158484971fb8308dacf8d1ce6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638050502240449300%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=%2BJa16pSkZiuk9e8YoYR6o%2F1TpNCg%2FozlGNLeRjrEGBc%3D&amp;reserved=0
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&amp;data=05%7C01%7Cabner.chang%40amd.com%7C4b16779158484971fb8308dacf8d1ce6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638050502240449300%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=QCmY7omz67ulLnn9Xi%2Br%2FudQrhRNCOU6qJmjHDQ9rHE%3D&amp;reserved=0 [abner.chang@amd.com]
-=-=-=-=-=-=-=-=-=-=-=-