[RFC] On the way C libraries (don't) expose all functions defined in coding guideline rules A.4 and A.5

[aescolar]

Background:

1. Today the Zephyr coding guidelines A.4 & A.5 specify what functions can be used in the kernel and in-tree.
   1.1) Most of these functions are part of the base C11 standard, two are (POSIX.1-2008) extensions.
2. Some developers have the understanding that this requires the used C libraries to expose these functions without the need for the user files (C / cmake) to define any SOURCE macros. Picolibc and minimallibc do this. Newlib, the integration with the host C library for the native targets, or other libraries out there don't.
3. Checkpatch today has a simple check of A.4 and A.5 by warning on definitions of SOURCE macros in .c files. This has two problems: 4.1) it does not detect definitions in build files. 4.2) It blocks legitimate uses of these macros.

This RFC does NOT try to discuss what should be in 1).
If focuses on 2)

Clarification needed

Has there been any kind of authoritative decision on 2) (i.e. TSC vote), or is this just an understanding reached in some PR discussion by a subset of developers?

Discussion on 2)

Today PicolibC ensures that these 2 extra functions prototypes are defined by treating Zephyr specially, with extra Zephyr specific ifdefs to detect and expose them. Newlib does not, and one cannot expect other C libraries the user may use to provide these.
The expectation that these 2 are exposed, sets a weird requirement on the C library integration
(See #67040 (comment) , #67040 (comment) , #67040 (comment) )

Under the assumption that 2) is the way forward, a simple way to work around this issue for C libraries without special Zephyr handling is to define globally the _POSIX_C_SOURCE macro. This is what PicolibC did before, and what #67040 proposed doing for Newlib.
There is the minor disadvantage of doing this, that we can cause this macro to be redefined, causing a redefinition warning, either in tree or in users code.

So far the only issue with assuming that 2) is not the way forward, is that the _POSIX_C_SOURCE macro needs to be defined where those 2 functions are used. As such there is no drawback of this beyond a) _POSIX_C_SOURCE needs to be defined in those files , and b) That checkpatch is triggered if that is done in the C source files themselves.
a) is something one would need to do with another OS or C library, so something users should expect and technically correct. Avoiding to do this due to that checkpatch check seems the wrong reason, as checkpatch detection of this rules violation is anyhow both very limited and overeager.

Proposal

• C libraries are not expected to expose the extra functions defined in A.4 and A.5 by default
• Source code in tree defines the required _POSIX_C_SOURCE macro locally (in the C file or build for that file/library) where it uses these extra functions.
• Checkpatch detection of the _POSIX_C_SOURCE macro usage in .c files is fixed (i.e. does not warn for this use when only these 2 functions are used).
• To avoid people assuming the oposite, coding guidelines rules A.4 & A.5 would state that C libraries may require _POSIX_C_SOURCE be set for those 2 functions prototypes to be visible.

Benefit of the proposal:

• We stop requiring extra integration and adding complexity around integration of C libraries. We just do what we would be expected to do and is technically correct. This would simplify the integration of other C libraries both in and out of tree, and avoid issues derived from the workarounds that extra integration requires (There is no nice way of solving this issue without modifying the C library source).
• PicolibC can stop treating Zephyr specially if desired (i.e. remove _ZEPHYR_VISIBLE)
• Checkpatch check for this rule needs refining anyhow, as today's detection is faulty both in false positives and false negatives.

References:

• https://docs.zephyrproject.org/latest/contribute/coding_guidelines/index.html#rule-a-4-c-standard-library-usage-restrictions-in-zephyr-kernel
• https://docs.zephyrproject.org/latest/contribute/coding_guidelines/index.html#rule-a-5-c-standard-library-usage-restrictions-in-zephyr-codebase
• Add coding guidelines on C standard library function usage #57598
• Remove explicit -D_POSIX_C_SOURCE global compiler options when building with picolibc #67040
• Several fixes around POSIX_SOURCE, remove global definitions of POSIX_SOURCE #67052
• Make picolibc the default C library for Zephyr #49922
  • Specifically this part of the discussion Make picolibc the default C library for Zephyr #49922 (comment)
• scripts/checkpatch: Check for patches adding #defines for libc APIs #60487
• libc: newlib/picolibc: define prototypes for non-ansi C functions #57619

Additional notes

Please do not use this RFC to discuss what functions should be part of A.4 & A.5. Create other RFCs for that.

Edit: In proposal replaced "SOURCE macros" with "_POSIX_C_SOURCE macro" as this is the only one I really intended to discuss about here.

[aescolar]

CC @stephanosio @keith-packard @cfriedt @henrikbrixandersen @kartben

[cfriedt]

Some developers have the understanding that this requires the used C libraries to expose these functions without the need for the user files (C / cmake) to define any SOURCE macros. Picolibc and minimallibc do this. Newlib, the integration with the host C library for the native targets, or other libraries out there don't.

Wow - this sounds an awful lot like someone is being told what they think, rather than their opinion simply being asked of them..

Can you tag the developers you are referring to, here? So that maybe they could share their opinion themselves?

[aescolar]

Can you tag the developers you are referring to

They have been tagged. You are welcome to elaborate on your point of view. This is an RFC.

[keith-packard]

I've modified #67040 so that it makes newlib expose the additional APIs by wrapping string.h. It seems like we should be able to do the same for any other C library? Even the native C library?

[aescolar]

I've modified #67040 so that it makes newlib expose the additional APIs by wrapping string.h. It seems like we should be able to do the same for any other C library? Even the native C library?

Thanks @keith-packard , but that assumes an outcome for this RFC which is not given. I would prefer if we could discuss here first if we should expect C libraries integration to do this. If you think that it should be yes, you are more than welcome to motivate it.

[aescolar]

It seems like we should be able to do the same for any other C library? Even the native C library?

For the libraries Zephyr provides (either as source or as part of the SDK) we can. Though we create a source relationship which architecturally I would think is best to avoid. (i.e. "Is it good that any file in the OS side knows about C library implementation details instead of just relaying on the standard way of configuring it?")
An important thing to note here is that we do not necessarily know what library the user uses (Zephyr is used by some with their own C library), so as such that creates that problem for those users.
(For the native C library, although typically that is glibc I don't think we can assume it in general. And even for glibc we cannot know the version)
So as such, the complexity we add both to ourselves and our users would be an argument against assuming the C library needs to expose these 2 extra function prototypes by default.
To me, it seems the main argument for exposing these 2 prototypes is one of minor convenience for developers and avoiding tripping checkpatch (if the users adds _POSIX_C_SOURCE to expose them in .c files, instead of in cmake).
So I lean quite strongly towards not assuming the C library is meant to expose these by default, neither by special treatment of Zephyr in the C library, or its integration.

[aescolar]

As an attempt to collect this conversation in one place, here a verbatim copy of @stephanosio comment: #68381 (comment)

---

Just reiterating the bits from the conversation I had offline with @aescolar for the record

If the conclusion of #68278 is that C libraries should indeed expose these 2 functions by default

I would prefer not to clutter individual Zephyr source files with the libc feature test macros because:

1. It is too "noisy."
2. Enforcing the POSIX C extension usage throughout Zephyr codebase becomes much more difficult because defining _POSIX_C_SOURCE pulls in a lot more than just strnlen and some other functions from the POSIX standard that we allow in the Coding Guidelines (i.e. we basically have to manually list and block all POSIX C extension functions in the CI compliance check, as opposed to just having one check that blocks _POSIX_C_SOURCE).
3. Defining _POSIX_C_SOURCE in individual Zephyr source files sets the pattern for defining libc feature test macros in individual Zephyr source files. While _POSIX_C_SOURCE is a rather special case being part of the POSIX standard, other common feature test macros like _GNU_SOURCE and _BSD_SOURCE are not part of any "standards" and their (equivalent) names and interpretations may vary across different libcs (at this time, we only have two POSIX functions in the allowed non-ISO C function list; but, it is foreseeable that the list will grow in the future).

It is a bit not nice though to have the OS be so aware of newlib implementation details (so we couple to some degree the newlib version from the SDK with the Zephyr version). Architecturally it does not feel nice. Though this concern may be a bit theoretical, and I do not have in my mind a specially better option.

I think libc awareness from the OS side and vice versa is something inevitable for optimal integration between the two (e.g. things like thread safety, reentrancy, atomics and threads all require awareness from both sides). Without it, the libc integration would not be "complete."

[cfriedt]

2. Some developers have the understanding that this requires the used C libraries to expose these functions without the need for the user files (C / cmake) to define any SOURCE macros. Picolibc and minimallibc do this. Newlib, the integration with the host C library for the native targets, or other libraries out there don't.

Maybe I've misunderstood, but I do not believe point 2 accurately describes picolibc.

It also seems that nobody has come forward to confirm their understanding is as described above otherwise.

Just a suggestion, but it might be better to reword point 2 so that it only includes facts.

E.g., if I were to reword point number 2, I would write

2. the minimal libc does not conditionally declare POSIX functions based on application conformance macros