public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
@ 2024-10-11  1:20 Rebecca Cran
  2024-10-11  3:41 ` Sean
  2024-10-15 21:12 ` Michael D Kinney
  0 siblings, 2 replies; 17+ messages in thread
From: Rebecca Cran @ 2024-10-11  1:20 UTC (permalink / raw)
  To: Michael Kinney, devel; +Cc: Rebecca Cran

A while ago a decision was made on the edk2-devel mailing list that
the `STATIC` EDK2 type should be replaced with the C keyword `static`.

Update the Coding Specification to match. While here, remove the
outdated section disallowing static functions since they're no longer
a problem.

Bump the revision to 2.4 and turn off the draft status.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
---
 5_source_files/54_code_file_structure.md    | 8 +-------
 5_source_files/56_declarations_and_types.md | 2 +-
 README.md                                   | 3 ++-
 book.json                                   | 4 ++--
 4 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/5_source_files/54_code_file_structure.md b/5_source_files/54_code_file_structure.md
index 0c4d6a2..6decc60 100644
--- a/5_source_files/54_code_file_structure.md
+++ b/5_source_files/54_code_file_structure.md
@@ -269,7 +269,7 @@ 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.
+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.
 
@@ -277,9 +277,3 @@ 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.
diff --git a/5_source_files/56_declarations_and_types.md b/5_source_files/56_declarations_and_types.md
index ec1803d..77e3ce9 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 is a violation of the coding convention.
 
 The corresponding EFI types must be used instead.
 
diff --git a/README.md b/README.md
index 77cfdc8..b543bcf 100644
--- a/README.md
+++ b/README.md
@@ -114,4 +114,5 @@ Copyright (c) 2006-2017, Intel Corporation. All rights reserved.
 |          | [#425](https://bugzilla.tianocore.org/show_bug.cgi?id=425) [CCS] clarify line breaking and indentation requirements for multi-line function calls |            |
 |          | [#1656](https://bugzilla.tianocore.org/show_bug.cgi?id=1656) Update all Wiki pages for the BSD+Patent license change with SPDX identifiers        |            |
 |          | [#607](https://bugzilla.tianocore.org/show_bug.cgi?id=607) Document code comment requirements for spurious variable assignments                   |            |
-| 2.3      | Add 4.2 Directory names section and update File names section for the guidelines of module directory and file naming|September 2022||
+| 2.3      | Add 4.2 Directory names section and update File names section for the guidelines of module directory and file naming                              |September 2022|
+| 2.4      | The use of the 'static' C keyword is now preferred over the EDK2 type 'STATIC'                                                                    |October 2024|
diff --git a/book.json b/book.json
index d112b26..3ec7a93 100644
--- a/book.json
+++ b/book.json
@@ -1,8 +1,8 @@
 {
   "variables" : {
-    "draft"   : "yes",
+    "draft"   : "no",
     "title"   : "EDK II C Coding Standards Specification",
-    "version" : "Revision 2.2"
+    "version" : "Revision 2.4"
   },
   "plugins": ["puml-aleung"],
   "pluginsConfig": {}
-- 
2.46.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120616): https://edk2.groups.io/g/devel/message/120616
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
  2024-10-11  1:20 [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` Rebecca Cran
@ 2024-10-11  3:41 ` Sean
  2024-10-11 16:47   ` Rebecca Cran
  2024-10-15 21:12 ` Michael D Kinney
  1 sibling, 1 reply; 17+ messages in thread
From: Sean @ 2024-10-11  3:41 UTC (permalink / raw)
  To: devel@edk2.groups.io, rebecca@bsdio.com, Michael Kinney,
	Oliver Smith-Denny
  Cc: Rebecca Cran

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

I think @Oliver Smith-Denny<mailto:osde@linux.microsoft.com> has proposed keeping the macro STATIC as a way to enable cleaner and easier unit tests. Did that get resolved?

Thanks
Sean
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Rebecca Cran <rebecca@bsdio.com>
Sent: Thursday, October 10, 2024 6:20:39 PM
To: Michael Kinney <michael.d.kinney@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Rebecca Cran <rebecca@bsdio.com>
Subject: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`

A while ago a decision was made on the edk2-devel mailing list that
the `STATIC` EDK2 type should be replaced with the C keyword `static`.

Update the Coding Specification to match. While here, remove the
outdated section disallowing static functions since they're no longer
a problem.

Bump the revision to 2.4 and turn off the draft status.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
---
 5_source_files/54_code_file_structure.md    | 8 +-------
 5_source_files/56_declarations_and_types.md | 2 +-
 README.md                                   | 3 ++-
 book.json                                   | 4 ++--
 4 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/5_source_files/54_code_file_structure.md b/5_source_files/54_code_file_structure.md
index 0c4d6a2..6decc60 100644
--- a/5_source_files/54_code_file_structure.md
+++ b/5_source_files/54_code_file_structure.md
@@ -269,7 +269,7 @@ 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.

+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.



@@ -277,9 +277,3 @@ 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.

diff --git a/5_source_files/56_declarations_and_types.md b/5_source_files/56_declarations_and_types.md
index ec1803d..77e3ce9 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 is a violation of the coding convention.



 The corresponding EFI types must be used instead.



diff --git a/README.md b/README.md
index 77cfdc8..b543bcf 100644
--- a/README.md
+++ b/README.md
@@ -114,4 +114,5 @@ Copyright (c) 2006-2017, Intel Corporation. All rights reserved.
 |          | [#425](https://bugzilla.tianocore.org/show_bug.cgi?id=425) [CCS] clarify line breaking and indentation requirements for multi-line function calls |            |

 |          | [#1656](https://bugzilla.tianocore.org/show_bug.cgi?id=1656) Update all Wiki pages for the BSD+Patent license change with SPDX identifiers        |            |

 |          | [#607](https://bugzilla.tianocore.org/show_bug.cgi?id=607) Document code comment requirements for spurious variable assignments                   |            |

-| 2.3      | Add 4.2 Directory names section and update File names section for the guidelines of module directory and file naming|September 2022||

+| 2.3      | Add 4.2 Directory names section and update File names section for the guidelines of module directory and file naming                              |September 2022|

+| 2.4      | The use of the 'static' C keyword is now preferred over the EDK2 type 'STATIC'                                                                    |October 2024|

diff --git a/book.json b/book.json
index d112b26..3ec7a93 100644
--- a/book.json
+++ b/book.json
@@ -1,8 +1,8 @@
 {

   "variables" : {

-    "draft"   : "yes",

+    "draft"   : "no",

     "title"   : "EDK II C Coding Standards Specification",

-    "version" : "Revision 2.2"

+    "version" : "Revision 2.4"

   },

   "plugins": ["puml-aleung"],

   "pluginsConfig": {}

--
2.46.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120616): https://edk2.groups.io/g/devel/message/120616
Mute This Topic: https://groups.io/mt/108941574/1686594
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [spbrogan@outlook.com]
-=-=-=-=-=-=




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120617): https://edk2.groups.io/g/devel/message/120617
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
  2024-10-11  3:41 ` Sean
@ 2024-10-11 16:47   ` Rebecca Cran
  2024-10-14 15:22     ` Oliver Smith-Denny
  0 siblings, 1 reply; 17+ messages in thread
From: Rebecca Cran @ 2024-10-11 16:47 UTC (permalink / raw)
  To: Sean Brogan, devel@edk2.groups.io, Michael Kinney,
	Oliver Smith-Denny

I don't know, but I saw a message from Mike recently saying that someone 
should use 'static' instead of 'STATIC' - so we need to make a clear 
decision one way or the other.


-- 
Rebecca


On 10/10/24 9:41 PM, Sean Brogan wrote:
> I think @Oliver Smith-Denny <mailto:osde@linux.microsoft.com> has 
> proposed keeping the macro STATIC as a way to enable cleaner and 
> easier unit tests. Did that get resolved?
>
> Thanks
> Sean
> ------------------------------------------------------------------------
> *From:* devel@edk2.groups.io <devel@edk2.groups.io> on behalf of 
> Rebecca Cran <rebecca@bsdio.com>
> *Sent:* Thursday, October 10, 2024 6:20:39 PM
> *To:* Michael Kinney <michael.d.kinney@intel.com>; 
> devel@edk2.groups.io <devel@edk2.groups.io>
> *Cc:* Rebecca Cran <rebecca@bsdio.com>
> *Subject:* [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] 
> Prefer use of `static` C keyword over EDK2 type `STATIC`
> A while ago a decision was made on the edk2-devel mailing list that
> the `STATIC` EDK2 type should be replaced with the C keyword `static`.
>
> Update the Coding Specification to match. While here, remove the
> outdated section disallowing static functions since they're no longer
> a problem.
>
> Bump the revision to 2.4 and turn off the draft status.
>
> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
> ---
>  5_source_files/54_code_file_structure.md    | 8 +-------
>  5_source_files/56_declarations_and_types.md | 2 +-
>  README.md                                   | 3 ++-
>  book.json                                   | 4 ++--
>  4 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/5_source_files/54_code_file_structure.md 
> b/5_source_files/54_code_file_structure.md
> index 0c4d6a2..6decc60 100644
> --- a/5_source_files/54_code_file_structure.md
> +++ b/5_source_files/54_code_file_structure.md
> @@ -269,7 +269,7 @@ 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.
>
> +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.
>
>
>
> @@ -277,9 +277,3 @@ 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.
>
> diff --git a/5_source_files/56_declarations_and_types.md 
> b/5_source_files/56_declarations_and_types.md
> index ec1803d..77e3ce9 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 is a 
> violation of the coding convention.
>
>
>
>  The corresponding EFI types must be used instead.
>
>
>
> diff --git a/README.md b/README.md
> index 77cfdc8..b543bcf 100644
> --- a/README.md
> +++ b/README.md
> @@ -114,4 +114,5 @@ Copyright (c) 2006-2017, Intel Corporation. All 
> rights reserved.
>  |          | 
> [#425](https://bugzilla.tianocore.org/show_bug.cgi?id=425) [CCS] 
> clarify line breaking and indentation requirements for multi-line 
> function calls |            |
>
>  |          | 
> [#1656](https://bugzilla.tianocore.org/show_bug.cgi?id=1656) Update 
> all Wiki pages for the BSD+Patent license change with SPDX 
> identifiers        |            |
>
>  |          | 
> [#607](https://bugzilla.tianocore.org/show_bug.cgi?id=607) Document 
> code comment requirements for spurious variable 
> assignments                   |            |
>
> -| 2.3      | Add 4.2 Directory names section and update File names 
> section for the guidelines of module directory and file 
> naming|September 2022||
>
> +| 2.3      | Add 4.2 Directory names section and update File names 
> section for the guidelines of module directory and file 
> naming                              |September 2022|
>
> +| 2.4      | The use of the 'static' C keyword is now preferred over 
> the EDK2 type 'STATIC' |October 2024|
>
> diff --git a/book.json b/book.json
> index d112b26..3ec7a93 100644
> --- a/book.json
> +++ b/book.json
> @@ -1,8 +1,8 @@
>  {
>
>    "variables" : {
>
> -    "draft"   : "yes",
>
> +    "draft"   : "no",
>
>      "title"   : "EDK II C Coding Standards Specification",
>
> -    "version" : "Revision 2.2"
>
> +    "version" : "Revision 2.4"
>
>    },
>
>    "plugins": ["puml-aleung"],
>
>    "pluginsConfig": {}
>
> -- 
> 2.46.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#120616): https://edk2.groups.io/g/devel/message/120616
> Mute This Topic: https://groups.io/mt/108941574/1686594
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [spbrogan@outlook.com]
> -=-=-=-=-=-=
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120618): https://edk2.groups.io/g/devel/message/120618
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
  2024-10-11 16:47   ` Rebecca Cran
@ 2024-10-14 15:22     ` Oliver Smith-Denny
  2024-10-14 16:47       ` Rebecca Cran
  0 siblings, 1 reply; 17+ messages in thread
From: Oliver Smith-Denny @ 2024-10-14 15:22 UTC (permalink / raw)
  To: devel, rebecca, Sean Brogan, Michael Kinney

On 10/11/2024 9:47 AM, Rebecca Cran wrote:
> I don't know, but I saw a message from Mike recently saying that someone 
> should use 'static' instead of 'STATIC' - so we need to make a clear 
> decision one way or the other.
> 
> 

The problem here is for GoogleTest. In CMocka, if you want to unit test
a static function, regardless of STATIC vs static, you can include the C
file under test in the CMocka test file. Not necessarily the most
elegant solution, but it works.

In GoogleTest, you often cannot directly include the C file in the
GoogleTest file, because C++ complains about many Cisms, mostly
our use of casting. And I definitely don't think we should update
the entire codebase to support compiling under C++ :). Mike, Sean,
and I had a discussion on a Project Mu PR where I undef'd STATIC
while building GoogleTest modules so that static functions could
be unit tested there, too. Also not the most elegant of solutions.

If we switch to "static", we lose the ability to unit test static
functions under GoogleTest in many cases. I don't know if GoogleTest
has some other solution for this, but I don't think so. It may be we
say, yep, that's the limitation of GoogleTest, add it to the
UnitTestFrameworkPkg README, if you need to unit test a static
function, do it in CMocka. But that seems lame, also.

I do believe that unit testing static functions is critical. We should
be doing interface level testing, as well, where static functions are
hidden from us, but there is great value in ensuring each piece of your
code is working as intended from the inside.

Mike, do you have further thoughts here?

Thanks,
Oliver



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120620): https://edk2.groups.io/g/devel/message/120620
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
  2024-10-14 15:22     ` Oliver Smith-Denny
@ 2024-10-14 16:47       ` Rebecca Cran
  2024-10-14 17:09         ` Oliver Smith-Denny
  0 siblings, 1 reply; 17+ messages in thread
From: Rebecca Cran @ 2024-10-14 16:47 UTC (permalink / raw)
  To: Oliver Smith-Denny, devel, Sean Brogan, Michael Kinney

On 10/14/24 9:22 AM, Oliver Smith-Denny wrote:
> In GoogleTest, you often cannot directly include the C file in the
> GoogleTest file, because C++ complains about many Cisms, mostly
> our use of casting.

Do you have an example of that?

Also, I've seen suggestions (I think on the FreeBSD mailing lists) of 
using C++ as "a better C". That is, not using any C++ specific features 
but building the code as C++ because apparently you get better code 
checking and diagnostics with it. Though you do lose some features like 
implicit casts from void* when using C++.


Rebecca



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120621): https://edk2.groups.io/g/devel/message/120621
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
  2024-10-14 16:47       ` Rebecca Cran
@ 2024-10-14 17:09         ` Oliver Smith-Denny
  0 siblings, 0 replies; 17+ messages in thread
From: Oliver Smith-Denny @ 2024-10-14 17:09 UTC (permalink / raw)
  To: devel, rebecca, Sean Brogan, Michael Kinney

On 10/14/2024 9:47 AM, Rebecca Cran wrote:
> On 10/14/24 9:22 AM, Oliver Smith-Denny wrote:
>> In GoogleTest, you often cannot directly include the C file in the
>> GoogleTest file, because C++ complains about many Cisms, mostly
>> our use of casting.
> 
> Do you have an example of that?

This was a little while back, so my only memory is of the Mu code I
updated the STATIC undef'ing for, one of these two:

https://github.com/microsoft/mu_plus/blob/dev/202405/AdvLoggerPkg/AdvLoggerOsConnectorPrm/GoogleTest/AdvLoggerOsConnectorPrmGoogleTest.cpp

https://github.com/microsoft/mu_plus/blob/dev/202405/AdvLoggerPkg/AdvLoggerOsConnectorPrm/Library/AdvLoggerOsConnectorPrmConfigLib/GoogleTest/AdvLoggerPrmConfigLibGoogleTest.cpp

We had issues in edk2 as well, I just don't remember specific examples.
It is more likely than not that if you try to write a GoogleTest and
include the C file you'll fail compilation.

> 
> Also, I've seen suggestions (I think on the FreeBSD mailing lists) of 
> using C++ as "a better C". That is, not using any C++ specific features 
> but building the code as C++ because apparently you get better code 
> checking and diagnostics with it. Though you do lose some features like 
> implicit casts from void* when using C++.

I would take umbrage with the statement that C++ is a better C :). I do
not think we gain value and in fact lose quite a lot of things,
name mangling, lots of churn, etc.

TBH, I have not been convinced of the value of adding C++ into edk2 with
GoogleTest, I think it has added complications and very few new edk2
unit tests. Hopefully platform owners are getting good usage out of it.

Oliver





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120622): https://edk2.groups.io/g/devel/message/120622
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
  2024-10-11  1:20 [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` Rebecca Cran
  2024-10-11  3:41 ` Sean
@ 2024-10-15 21:12 ` Michael D Kinney
  2024-10-21 14:49   ` Rebecca Cran
  1 sibling, 1 reply; 17+ messages in thread
From: Michael D Kinney @ 2024-10-15 21:12 UTC (permalink / raw)
  To: Rebecca Cran, devel@edk2.groups.io; +Cc: Kinney, Michael D

The draft status is never disabled in the master branch.

Please see the release process for documents.

https://github.com/tianocore-docs/edk2-TemplateSpecification/wiki/TianoCore-Documents-Releasing

Document changes should be made on the master branch, and then the release branch is created with no content changes.

Mike

> -----Original Message-----
> From: Rebecca Cran <rebecca@bsdio.com>
> Sent: Thursday, October 10, 2024 6:21 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> Cc: Rebecca Cran <rebecca@bsdio.com>
> Subject: [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of
> `static` C keyword over EDK2 type `STATIC`
> 
> A while ago a decision was made on the edk2-devel mailing list that
> the `STATIC` EDK2 type should be replaced with the C keyword `static`.
> 
> Update the Coding Specification to match. While here, remove the
> outdated section disallowing static functions since they're no longer
> a problem.
> 
> Bump the revision to 2.4 and turn off the draft status.
> 
> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
> ---
>  5_source_files/54_code_file_structure.md    | 8 +-------
>  5_source_files/56_declarations_and_types.md | 2 +-
>  README.md                                   | 3 ++-
>  book.json                                   | 4 ++--
>  4 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/5_source_files/54_code_file_structure.md
> b/5_source_files/54_code_file_structure.md
> index 0c4d6a2..6decc60 100644
> --- a/5_source_files/54_code_file_structure.md
> +++ b/5_source_files/54_code_file_structure.md
> @@ -269,7 +269,7 @@ 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.
> 
> +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.
> 
> 
> 
> @@ -277,9 +277,3 @@ 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.
> 
> diff --git a/5_source_files/56_declarations_and_types.md
> b/5_source_files/56_declarations_and_types.md
> index ec1803d..77e3ce9 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 is a violation
> of the coding convention.
> 
> 
> 
>  The corresponding EFI types must be used instead.
> 
> 
> 
> diff --git a/README.md b/README.md
> index 77cfdc8..b543bcf 100644
> --- a/README.md
> +++ b/README.md
> @@ -114,4 +114,5 @@ Copyright (c) 2006-2017, Intel Corporation. All
> rights reserved.
>  |          | [#425](https://bugzilla.tianocore.org/show_bug.cgi?id=425)
> [CCS] clarify line breaking and indentation requirements for multi-line
> function calls |            |
> 
>  |          |
> [#1656](https://bugzilla.tianocore.org/show_bug.cgi?id=1656) Update all
> Wiki pages for the BSD+Patent license change with SPDX identifiers
> |            |
> 
>  |          | [#607](https://bugzilla.tianocore.org/show_bug.cgi?id=607)
> Document code comment requirements for spurious variable assignments
> |            |
> 
> -| 2.3      | Add 4.2 Directory names section and update File names
> section for the guidelines of module directory and file naming|September
> 2022||
> 
> +| 2.3      | Add 4.2 Directory names section and update File names
> section for the guidelines of module directory and file naming
> |September 2022|
> 
> +| 2.4      | The use of the 'static' C keyword is now preferred over
> the EDK2 type 'STATIC'
> |October 2024|
> 
> diff --git a/book.json b/book.json
> index d112b26..3ec7a93 100644
> --- a/book.json
> +++ b/book.json
> @@ -1,8 +1,8 @@
>  {
> 
>    "variables" : {
> 
> -    "draft"   : "yes",
> 
> +    "draft"   : "no",
> 
>      "title"   : "EDK II C Coding Standards Specification",
> 
> -    "version" : "Revision 2.2"
> 
> +    "version" : "Revision 2.4"
> 
>    },
> 
>    "plugins": ["puml-aleung"],
> 
>    "pluginsConfig": {}
> 
> --
> 2.46.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120625): https://edk2.groups.io/g/devel/message/120625
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
  2024-10-15 21:12 ` Michael D Kinney
@ 2024-10-21 14:49   ` Rebecca Cran
  2024-10-21 15:46     ` Michael D Kinney
  2024-10-21 19:42     ` Pedro Falcato
  0 siblings, 2 replies; 17+ messages in thread
From: Rebecca Cran @ 2024-10-21 14:49 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io

Thanks, I'll fix it.

Could you confirm whether the change from STATIC to static is something 
we want to go ahead with, or do we want to keep STATIC to allow 
GoogleTest to work?


-- 
Rebecca


On 10/15/24 3:12 PM, Kinney, Michael D wrote:
> The draft status is never disabled in the master branch.
>
> Please see the release process for documents.
>
> https://github.com/tianocore-docs/edk2-TemplateSpecification/wiki/TianoCore-Documents-Releasing
>
> Document changes should be made on the master branch, and then the release branch is created with no content changes.
>
> Mike
>
>> -----Original Message-----
>> From: Rebecca Cran <rebecca@bsdio.com>
>> Sent: Thursday, October 10, 2024 6:21 PM
>> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
>> Cc: Rebecca Cran <rebecca@bsdio.com>
>> Subject: [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of
>> `static` C keyword over EDK2 type `STATIC`
>>
>> A while ago a decision was made on the edk2-devel mailing list that
>> the `STATIC` EDK2 type should be replaced with the C keyword `static`.
>>
>> Update the Coding Specification to match. While here, remove the
>> outdated section disallowing static functions since they're no longer
>> a problem.
>>
>> Bump the revision to 2.4 and turn off the draft status.
>>
>> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
>> ---
>>   5_source_files/54_code_file_structure.md    | 8 +-------
>>   5_source_files/56_declarations_and_types.md | 2 +-
>>   README.md                                   | 3 ++-
>>   book.json                                   | 4 ++--
>>   4 files changed, 6 insertions(+), 11 deletions(-)
>>
>> diff --git a/5_source_files/54_code_file_structure.md
>> b/5_source_files/54_code_file_structure.md
>> index 0c4d6a2..6decc60 100644
>> --- a/5_source_files/54_code_file_structure.md
>> +++ b/5_source_files/54_code_file_structure.md
>> @@ -269,7 +269,7 @@ 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.
>>
>> +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.
>>
>>
>>
>> @@ -277,9 +277,3 @@ 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.
>>
>> diff --git a/5_source_files/56_declarations_and_types.md
>> b/5_source_files/56_declarations_and_types.md
>> index ec1803d..77e3ce9 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 is a violation
>> of the coding convention.
>>
>>
>>
>>   The corresponding EFI types must be used instead.
>>
>>
>>
>> diff --git a/README.md b/README.md
>> index 77cfdc8..b543bcf 100644
>> --- a/README.md
>> +++ b/README.md
>> @@ -114,4 +114,5 @@ Copyright (c) 2006-2017, Intel Corporation. All
>> rights reserved.
>>   |          | [#425](https://bugzilla.tianocore.org/show_bug.cgi?id=425)
>> [CCS] clarify line breaking and indentation requirements for multi-line
>> function calls |            |
>>
>>   |          |
>> [#1656](https://bugzilla.tianocore.org/show_bug.cgi?id=1656) Update all
>> Wiki pages for the BSD+Patent license change with SPDX identifiers
>> |            |
>>
>>   |          | [#607](https://bugzilla.tianocore.org/show_bug.cgi?id=607)
>> Document code comment requirements for spurious variable assignments
>> |            |
>>
>> -| 2.3      | Add 4.2 Directory names section and update File names
>> section for the guidelines of module directory and file naming|September
>> 2022||
>>
>> +| 2.3      | Add 4.2 Directory names section and update File names
>> section for the guidelines of module directory and file naming
>> |September 2022|
>>
>> +| 2.4      | The use of the 'static' C keyword is now preferred over
>> the EDK2 type 'STATIC'
>> |October 2024|
>>
>> diff --git a/book.json b/book.json
>> index d112b26..3ec7a93 100644
>> --- a/book.json
>> +++ b/book.json
>> @@ -1,8 +1,8 @@
>>   {
>>
>>     "variables" : {
>>
>> -    "draft"   : "yes",
>>
>> +    "draft"   : "no",
>>
>>       "title"   : "EDK II C Coding Standards Specification",
>>
>> -    "version" : "Revision 2.2"
>>
>> +    "version" : "Revision 2.4"
>>
>>     },
>>
>>     "plugins": ["puml-aleung"],
>>
>>     "pluginsConfig": {}
>>
>> --
>> 2.46.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120633): https://edk2.groups.io/g/devel/message/120633
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
  2024-10-21 14:49   ` Rebecca Cran
@ 2024-10-21 15:46     ` Michael D Kinney
  2024-10-21 19:42     ` Pedro Falcato
  1 sibling, 0 replies; 17+ messages in thread
From: Michael D Kinney @ 2024-10-21 15:46 UTC (permalink / raw)
  To: Rebecca Cran, devel@edk2.groups.io; +Cc: Kinney, Michael D

This needs further discussion.

Before GoogleTest was added, there was a clear direction to move from STATIC->static.

The issue at hand is really a C++/C compatibility issue. We need to enumerate the
full set of issues and see if there are reasonable ways to address.  

Mike

> -----Original Message-----
> From: Rebecca Cran <rebecca@bsdio.com>
> Sent: Monday, October 21, 2024 7:49 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> Subject: Re: [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use
> of `static` C keyword over EDK2 type `STATIC`
> 
> Thanks, I'll fix it.
> 
> Could you confirm whether the change from STATIC to static is something
> we want to go ahead with, or do we want to keep STATIC to allow
> GoogleTest to work?
> 
> 
> --
> Rebecca
> 
> 
> On 10/15/24 3:12 PM, Kinney, Michael D wrote:
> > The draft status is never disabled in the master branch.
> >
> > Please see the release process for documents.
> >
> > https://github.com/tianocore-docs/edk2-
> TemplateSpecification/wiki/TianoCore-Documents-Releasing
> >
> > Document changes should be made on the master branch, and then the
> release branch is created with no content changes.
> >
> > Mike
> >
> >> -----Original Message-----
> >> From: Rebecca Cran <rebecca@bsdio.com>
> >> Sent: Thursday, October 10, 2024 6:21 PM
> >> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> >> Cc: Rebecca Cran <rebecca@bsdio.com>
> >> Subject: [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of
> >> `static` C keyword over EDK2 type `STATIC`
> >>
> >> A while ago a decision was made on the edk2-devel mailing list that
> >> the `STATIC` EDK2 type should be replaced with the C keyword
> `static`.
> >>
> >> Update the Coding Specification to match. While here, remove the
> >> outdated section disallowing static functions since they're no longer
> >> a problem.
> >>
> >> Bump the revision to 2.4 and turn off the draft status.
> >>
> >> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
> >> ---
> >>   5_source_files/54_code_file_structure.md    | 8 +-------
> >>   5_source_files/56_declarations_and_types.md | 2 +-
> >>   README.md                                   | 3 ++-
> >>   book.json                                   | 4 ++--
> >>   4 files changed, 6 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/5_source_files/54_code_file_structure.md
> >> b/5_source_files/54_code_file_structure.md
> >> index 0c4d6a2..6decc60 100644
> >> --- a/5_source_files/54_code_file_structure.md
> >> +++ b/5_source_files/54_code_file_structure.md
> >> @@ -269,7 +269,7 @@ 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.
> >>
> >> +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.
> >>
> >>
> >>
> >> @@ -277,9 +277,3 @@ 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.
> >>
> >> diff --git a/5_source_files/56_declarations_and_types.md
> >> b/5_source_files/56_declarations_and_types.md
> >> index ec1803d..77e3ce9 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 is a
> violation
> >> of the coding convention.
> >>
> >>
> >>
> >>   The corresponding EFI types must be used instead.
> >>
> >>
> >>
> >> diff --git a/README.md b/README.md
> >> index 77cfdc8..b543bcf 100644
> >> --- a/README.md
> >> +++ b/README.md
> >> @@ -114,4 +114,5 @@ Copyright (c) 2006-2017, Intel Corporation. All
> >> rights reserved.
> >>   |          |
> [#425](https://bugzilla.tianocore.org/show_bug.cgi?id=425)
> >> [CCS] clarify line breaking and indentation requirements for multi-
> line
> >> function calls |            |
> >>
> >>   |          |
> >> [#1656](https://bugzilla.tianocore.org/show_bug.cgi?id=1656) Update
> all
> >> Wiki pages for the BSD+Patent license change with SPDX identifiers
> >> |            |
> >>
> >>   |          |
> [#607](https://bugzilla.tianocore.org/show_bug.cgi?id=607)
> >> Document code comment requirements for spurious variable assignments
> >> |            |
> >>
> >> -| 2.3      | Add 4.2 Directory names section and update File names
> >> section for the guidelines of module directory and file
> naming|September
> >> 2022||
> >>
> >> +| 2.3      | Add 4.2 Directory names section and update File names
> >> section for the guidelines of module directory and file naming
> >> |September 2022|
> >>
> >> +| 2.4      | The use of the 'static' C keyword is now preferred over
> >> the EDK2 type 'STATIC'
> >> |October 2024|
> >>
> >> diff --git a/book.json b/book.json
> >> index d112b26..3ec7a93 100644
> >> --- a/book.json
> >> +++ b/book.json
> >> @@ -1,8 +1,8 @@
> >>   {
> >>
> >>     "variables" : {
> >>
> >> -    "draft"   : "yes",
> >>
> >> +    "draft"   : "no",
> >>
> >>       "title"   : "EDK II C Coding Standards Specification",
> >>
> >> -    "version" : "Revision 2.2"
> >>
> >> +    "version" : "Revision 2.4"
> >>
> >>     },
> >>
> >>     "plugins": ["puml-aleung"],
> >>
> >>     "pluginsConfig": {}
> >>
> >> --
> >> 2.46.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120634): https://edk2.groups.io/g/devel/message/120634
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
  2024-10-21 14:49   ` Rebecca Cran
  2024-10-21 15:46     ` Michael D Kinney
@ 2024-10-21 19:42     ` Pedro Falcato
  2024-10-21 20:05       ` Oliver Smith-Denny
  1 sibling, 1 reply; 17+ messages in thread
From: Pedro Falcato @ 2024-10-21 19:42 UTC (permalink / raw)
  To: devel, rebecca; +Cc: Kinney, Michael D

On Mon, Oct 21, 2024 at 3:49 PM Rebecca Cran via groups.io
<rebecca=bsdio.com@groups.io> wrote:
>
> Thanks, I'll fix it.
>
> Could you confirm whether the change from STATIC to static is something
> we want to go ahead with, or do we want to keep STATIC to allow
> GoogleTest to work?

You don't need STATIC, doing stuff like -Dstatic= (or just #define
static in C code) Just Works. For GCC at least.

proof of horribleness: https://godbolt.org/z/EvMd6hev8

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120637): https://edk2.groups.io/g/devel/message/120637
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
  2024-10-21 19:42     ` Pedro Falcato
@ 2024-10-21 20:05       ` Oliver Smith-Denny
  2024-10-21 20:21         ` Pedro Falcato
  0 siblings, 1 reply; 17+ messages in thread
From: Oliver Smith-Denny @ 2024-10-21 20:05 UTC (permalink / raw)
  To: devel, pedro.falcato, rebecca; +Cc: Kinney, Michael D

On 10/21/2024 12:42 PM, Pedro Falcato wrote:
> On Mon, Oct 21, 2024 at 3:49 PM Rebecca Cran via groups.io
> <rebecca=bsdio.com@groups.io> wrote:
>>
>> Thanks, I'll fix it.
>>
>> Could you confirm whether the change from STATIC to static is something
>> we want to go ahead with, or do we want to keep STATIC to allow
>> GoogleTest to work?
> 
> You don't need STATIC, doing stuff like -Dstatic= (or just #define
> static in C code) Just Works. For GCC at least.
> 
> proof of horribleness: https://godbolt.org/z/EvMd6hev8
> 

The issue here is that C uses one keyword for two distinct things:
file private members and local variables that keep state across calls,
i.e. real static variables.

If you only have static, then #define static <nothing>, you'll mess up
the local variable case under test.

Don't get me wrong, I think all of this tinkering is horribleness and I
think that mixing languages often brings these kind of compromises,
which is unfortunate.

I'm not advocating we introduce these kind of hacks, but I do think we
should be able to unit test static functions, so I'd like to see a
solution for that, which again, might just be CMocka.

Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120638): https://edk2.groups.io/g/devel/message/120638
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
  2024-10-21 20:05       ` Oliver Smith-Denny
@ 2024-10-21 20:21         ` Pedro Falcato
  2024-10-21 20:26           ` Oliver Smith-Denny
  0 siblings, 1 reply; 17+ messages in thread
From: Pedro Falcato @ 2024-10-21 20:21 UTC (permalink / raw)
  To: Oliver Smith-Denny; +Cc: devel, rebecca, Kinney, Michael D

On Mon, Oct 21, 2024 at 9:05 PM Oliver Smith-Denny
<osde@linux.microsoft.com> wrote:
>
> On 10/21/2024 12:42 PM, Pedro Falcato wrote:
> > On Mon, Oct 21, 2024 at 3:49 PM Rebecca Cran via groups.io
> > <rebecca=bsdio.com@groups.io> wrote:
> >>
> >> Thanks, I'll fix it.
> >>
> >> Could you confirm whether the change from STATIC to static is something
> >> we want to go ahead with, or do we want to keep STATIC to allow
> >> GoogleTest to work?
> >
> > You don't need STATIC, doing stuff like -Dstatic= (or just #define
> > static in C code) Just Works. For GCC at least.
> >
> > proof of horribleness: https://godbolt.org/z/EvMd6hev8
> >
>
> The issue here is that C uses one keyword for two distinct things:
> file private members and local variables that keep state across calls,
> i.e. real static variables.

That's an interesting problem, but I'm afraid the current EDK2 coding
style would recommend STATIC for local variables too.

https://tianocore-docs.github.io/edk2-CCodingStandardsSpecification/release-2.20/5_source_files/56_declarations_and_types.html#56-declarations-and-types
says:

5.6.1.2 The use of int, unsigned, char, void, static, long is a
violation of the coding convention.

so:

INT Foo()
{
    STATIC INT Variable = 0;
    return Variable++;
}

would be the sanctioned way to pull off this kind of stuff. So STATIC
gains us nothing (neither does INT, LONG, CHAR, VOID, or CONST).

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120639): https://edk2.groups.io/g/devel/message/120639
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
  2024-10-21 20:21         ` Pedro Falcato
@ 2024-10-21 20:26           ` Oliver Smith-Denny
  2024-10-21 20:35             ` Michael D Kinney
  0 siblings, 1 reply; 17+ messages in thread
From: Oliver Smith-Denny @ 2024-10-21 20:26 UTC (permalink / raw)
  To: devel, pedro.falcato; +Cc: rebecca, Kinney, Michael D

On 10/21/2024 1:21 PM, Pedro Falcato wrote:
> On Mon, Oct 21, 2024 at 9:05 PM Oliver Smith-Denny
> <osde@linux.microsoft.com> wrote:
>>
>> On 10/21/2024 12:42 PM, Pedro Falcato wrote:
>>> On Mon, Oct 21, 2024 at 3:49 PM Rebecca Cran via groups.io
>>> <rebecca=bsdio.com@groups.io> wrote:
>>>>
>>>> Thanks, I'll fix it.
>>>>
>>>> Could you confirm whether the change from STATIC to static is something
>>>> we want to go ahead with, or do we want to keep STATIC to allow
>>>> GoogleTest to work?
>>>
>>> You don't need STATIC, doing stuff like -Dstatic= (or just #define
>>> static in C code) Just Works. For GCC at least.
>>>
>>> proof of horribleness: https://godbolt.org/z/EvMd6hev8
>>>
>>
>> The issue here is that C uses one keyword for two distinct things:
>> file private members and local variables that keep state across calls,
>> i.e. real static variables.
> 
> That's an interesting problem, but I'm afraid the current EDK2 coding
> style would recommend STATIC for local variables too.
> 
> https://tianocore-docs.github.io/edk2-CCodingStandardsSpecification/release-2.20/5_source_files/56_declarations_and_types.html#56-declarations-and-types
> says:
> 
> 5.6.1.2 The use of int, unsigned, char, void, static, long is a
> violation of the coding convention.
> 
> so:
> 
> INT Foo()
> {
>      STATIC INT Variable = 0;
>      return Variable++;
> }
> 
> would be the sanctioned way to pull off this kind of stuff. So STATIC
> gains us nothing (neither does INT, LONG, CHAR, VOID, or CONST).
> 

Yeah, so what we did in Mu was leverage the fact that static locals are
relatively uncommon and code coverage is extremely low, so that there 
was only one current instance of a static local under test. And we
changed that to lowercase static. So now STATIC bought us that we
could undef it when building HOST_APPLICATIONs.

Was I happy with that approach? No. But, we were trying to evaluate
the usefulness of GoogleTest and this was one issue that had to be
fixed in order for us to use it.

Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120640): https://edk2.groups.io/g/devel/message/120640
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
  2024-10-21 20:26           ` Oliver Smith-Denny
@ 2024-10-21 20:35             ` Michael D Kinney
  2024-10-21 20:37               ` Oliver Smith-Denny
  0 siblings, 1 reply; 17+ messages in thread
From: Michael D Kinney @ 2024-10-21 20:35 UTC (permalink / raw)
  To: Oliver Smith-Denny, devel@edk2.groups.io, pedro.falcato@gmail.com
  Cc: rebecca@bsdio.com, Kinney, Michael D

Would it help if code was modified to completely remove the use of 
static in local variables and enforce that going forward and limit
use of static to functions and global variables?

Mike

> -----Original Message-----
> From: Oliver Smith-Denny <osde@linux.microsoft.com>
> Sent: Monday, October 21, 2024 1:26 PM
> To: devel@edk2.groups.io; pedro.falcato@gmail.com
> Cc: rebecca@bsdio.com; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1]
> Prefer use of `static` C keyword over EDK2 type `STATIC`
> 
> On 10/21/2024 1:21 PM, Pedro Falcato wrote:
> > On Mon, Oct 21, 2024 at 9:05 PM Oliver Smith-Denny
> > <osde@linux.microsoft.com> wrote:
> >>
> >> On 10/21/2024 12:42 PM, Pedro Falcato wrote:
> >>> On Mon, Oct 21, 2024 at 3:49 PM Rebecca Cran via groups.io
> >>> <rebecca=bsdio.com@groups.io> wrote:
> >>>>
> >>>> Thanks, I'll fix it.
> >>>>
> >>>> Could you confirm whether the change from STATIC to static is
> something
> >>>> we want to go ahead with, or do we want to keep STATIC to allow
> >>>> GoogleTest to work?
> >>>
> >>> You don't need STATIC, doing stuff like -Dstatic= (or just #define
> >>> static in C code) Just Works. For GCC at least.
> >>>
> >>> proof of horribleness: https://godbolt.org/z/EvMd6hev8
> >>>
> >>
> >> The issue here is that C uses one keyword for two distinct things:
> >> file private members and local variables that keep state across
> calls,
> >> i.e. real static variables.
> >
> > That's an interesting problem, but I'm afraid the current EDK2 coding
> > style would recommend STATIC for local variables too.
> >
> > https://tianocore-docs.github.io/edk2-
> CCodingStandardsSpecification/release-
> 2.20/5_source_files/56_declarations_and_types.html#56-declarations-and-
> types
> > says:
> >
> > 5.6.1.2 The use of int, unsigned, char, void, static, long is a
> > violation of the coding convention.
> >
> > so:
> >
> > INT Foo()
> > {
> >      STATIC INT Variable = 0;
> >      return Variable++;
> > }
> >
> > would be the sanctioned way to pull off this kind of stuff. So STATIC
> > gains us nothing (neither does INT, LONG, CHAR, VOID, or CONST).
> >
> 
> Yeah, so what we did in Mu was leverage the fact that static locals are
> relatively uncommon and code coverage is extremely low, so that there
> was only one current instance of a static local under test. And we
> changed that to lowercase static. So now STATIC bought us that we
> could undef it when building HOST_APPLICATIONs.
> 
> Was I happy with that approach? No. But, we were trying to evaluate
> the usefulness of GoogleTest and this was one issue that had to be
> fixed in order for us to use it.
> 
> Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120641): https://edk2.groups.io/g/devel/message/120641
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
  2024-10-21 20:35             ` Michael D Kinney
@ 2024-10-21 20:37               ` Oliver Smith-Denny
  2024-10-21 21:04                 ` Michael D Kinney
  0 siblings, 1 reply; 17+ messages in thread
From: Oliver Smith-Denny @ 2024-10-21 20:37 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io, pedro.falcato@gmail.com
  Cc: rebecca@bsdio.com

On 10/21/2024 1:35 PM, Kinney, Michael D wrote:
> Would it help if code was modified to completely remove the use of
> static in local variables and enforce that going forward and limit
> use of static to functions and global variables?
> 

We'd have to test further, but I believe that would allow Pedro's
scheme of simply `#define static <nothing>` to work. I ran a
quick godbolt test and it seems MSVC is also okay with that.

Oliver



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120642): https://edk2.groups.io/g/devel/message/120642
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
  2024-10-21 20:37               ` Oliver Smith-Denny
@ 2024-10-21 21:04                 ` Michael D Kinney
  2024-10-21 21:11                   ` Pedro Falcato
  0 siblings, 1 reply; 17+ messages in thread
From: Michael D Kinney @ 2024-10-21 21:04 UTC (permalink / raw)
  To: Oliver Smith-Denny, devel@edk2.groups.io, pedro.falcato@gmail.com
  Cc: rebecca@bsdio.com, Kinney, Michael D

I recall clang having more restrictions on redefining keywords.

Mike

> -----Original Message-----
> From: Oliver Smith-Denny <osde@linux.microsoft.com>
> Sent: Monday, October 21, 2024 1:38 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io; pedro.falcato@gmail.com
> Cc: rebecca@bsdio.com
> Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1]
> Prefer use of `static` C keyword over EDK2 type `STATIC`
> 
> On 10/21/2024 1:35 PM, Kinney, Michael D wrote:
> > Would it help if code was modified to completely remove the use of
> > static in local variables and enforce that going forward and limit
> > use of static to functions and global variables?
> >
> 
> We'd have to test further, but I believe that would allow Pedro's
> scheme of simply `#define static <nothing>` to work. I ran a
> quick godbolt test and it seems MSVC is also okay with that.
> 
> Oliver



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120643): https://edk2.groups.io/g/devel/message/120643
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC`
  2024-10-21 21:04                 ` Michael D Kinney
@ 2024-10-21 21:11                   ` Pedro Falcato
  0 siblings, 0 replies; 17+ messages in thread
From: Pedro Falcato @ 2024-10-21 21:11 UTC (permalink / raw)
  To: Kinney, Michael D
  Cc: Oliver Smith-Denny, devel@edk2.groups.io, rebecca@bsdio.com

On Mon, Oct 21, 2024 at 10:04 PM Kinney, Michael D
<michael.d.kinney@intel.com> wrote:
>
> I recall clang having more restrictions on redefining keywords.

Clang has restrictions on redefining macros and builtin functions. It
appears to not have any issues doing horrible unspeakable things with
the C keywords and the C preprocessor: https://godbolt.org/z/eT6hWsEG5

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120644): https://edk2.groups.io/g/devel/message/120644
Mute This Topic: https://groups.io/mt/108941574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2024-10-21 21:11 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-11  1:20 [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` Rebecca Cran
2024-10-11  3:41 ` Sean
2024-10-11 16:47   ` Rebecca Cran
2024-10-14 15:22     ` Oliver Smith-Denny
2024-10-14 16:47       ` Rebecca Cran
2024-10-14 17:09         ` Oliver Smith-Denny
2024-10-15 21:12 ` Michael D Kinney
2024-10-21 14:49   ` Rebecca Cran
2024-10-21 15:46     ` Michael D Kinney
2024-10-21 19:42     ` Pedro Falcato
2024-10-21 20:05       ` Oliver Smith-Denny
2024-10-21 20:21         ` Pedro Falcato
2024-10-21 20:26           ` Oliver Smith-Denny
2024-10-21 20:35             ` Michael D Kinney
2024-10-21 20:37               ` Oliver Smith-Denny
2024-10-21 21:04                 ` Michael D Kinney
2024-10-21 21:11                   ` Pedro Falcato

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox