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


Chang, Abner
 

[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@...>
Sent: Monday, November 28, 2022 11:41 PM
To: Ard Biesheuvel <ardb@...>; devel@edk2.groups.io; Kinney,
Michael D <michael.d.kinney@...>
Cc: Chang, Abner <Abner.Chang@...>; Ni, Ray <ray.ni@...>;
Gao, Liming <gaoliming@...>
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@...>
Sent: Monday, November 28, 2022 1:08 AM
To: devel@edk2.groups.io; Kinney, Michael D
<michael.d.kinney@...>
Cc: Chang, Abner <Abner.Chang@...>; Ni, Ray <ray.ni@...>;
Gao, Liming <gaoliming@...>
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@...> 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.


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@...>
Sent: Monday, November 28, 2022 1:08 AM
To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>
Cc: Chang, Abner <Abner.Chang@...>; Ni, Ray <ray.ni@...>; Gao, Liming <gaoliming@...>
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@...> 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.


Ard Biesheuvel
 

On Tue, 22 Nov 2022 at 19:10, Michael D Kinney
<michael.d.kinney@...> 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.


Chang, Abner
 

[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@...>; Ni, Ray <ray.ni@...>;
devel@edk2.groups.io; Gao, Liming <gaoliming@...>
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@...>
Sent: Monday, November 21, 2022 10:08 PM
To: Ni, Ray <ray.ni@...>; devel@edk2.groups.io; Kinney, Michael
D <michael.d.kinney@...>; Gao, Liming
<gaoliming@...>
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@...>
Sent: Tuesday, November 22, 2022 1:48 PM
To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@...>
Cc: Kinney, Michael D <michael.d.kinney@...>
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@...>; Kinney, Michael D
<michael.d.kinney@...>
Subject: [edk2-devel] [PATCH] edk II C Coding Standard: Remove
section
5.4.2.2 STATIC

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

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@...>
Cc: Ray Ni <ray.ni@...>
Cc: Michael D Kinney <michael.d.kinney@...>
---
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








Michael D Kinney
 

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@...>
Sent: Tuesday, November 22, 2022 2:12 PM
To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>
Cc: Chang, Abner <Abner.Chang@...>; Ni, Ray <ray.ni@...>; Gao, Liming <gaoliming@...>
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@...> 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


Pedro Falcato
 

On Tue, Nov 22, 2022 at 6:10 PM Michael D Kinney <michael.d.kinney@...> 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


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@...>
Sent: Monday, November 21, 2022 10:08 PM
To: Ni, Ray <ray.ni@...>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Gao, Liming
<gaoliming@...>
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@...>
Sent: Tuesday, November 22, 2022 1:48 PM
To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@...>
Cc: Kinney, Michael D <michael.d.kinney@...>
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@...>; Kinney, Michael D
<michael.d.kinney@...>
Subject: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section
5.4.2.2 STATIC

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

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@...>
Cc: Ray Ni <ray.ni@...>
Cc: Michael D Kinney <michael.d.kinney@...>
---
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





Chang, Abner
 

[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@...>
Sent: Tuesday, November 22, 2022 1:48 PM
To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@...>
Cc: Kinney, Michael D <michael.d.kinney@...>
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@...>; Kinney, Michael D
<michael.d.kinney@...>
Subject: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section
5.4.2.2 STATIC

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

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@...>
Cc: Ray Ni <ray.ni@...>
Cc: Michael D Kinney <michael.d.kinney@...>
---
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





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@...>; Kinney, Michael D
<michael.d.kinney@...>
Subject: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section
5.4.2.2 STATIC

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

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@...>
Cc: Ray Ni <ray.ni@...>
Cc: Michael D Kinney <michael.d.kinney@...>
---
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





Chang, Abner
 

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

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@...>
Cc: Ray Ni <ray.ni@...>
Cc: Michael D Kinney <michael.d.kinney@...>
---
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