Skip to content

CODING_CONVENTIONS.md: Elaborate on C11 compliance#21140

Merged
MrKevinWeiss merged 1 commit intoRIOT-OS:masterfrom
maribu:CODING_CONVENTIONS.md/std-c-exceptions
Jan 21, 2025
Merged

CODING_CONVENTIONS.md: Elaborate on C11 compliance#21140
MrKevinWeiss merged 1 commit intoRIOT-OS:masterfrom
maribu:CODING_CONVENTIONS.md/std-c-exceptions

Conversation

@maribu
Copy link
Copy Markdown
Member

@maribu maribu commented Jan 19, 2025

Contribution description

This adds reasoning for requiring standard compliant C code, and lists generally accepted exceptions to this rule.

Testing procedure

Read the changes and reasoning.

Issues/PRs references

https://forum.riot-os.org/t/notes-virtual-maintainer-assembly-vma-2024-11/4428

@maribu maribu requested a review from jia200x as a code owner January 19, 2025 21:17
@github-actions github-actions bot added the Area: doc Area: Documentation label Jan 19, 2025
Comment on lines +73 to +74
- `__attribute__((used))`, `__attribute__((section("...")))`,
`__attribute__((weak))`, and `__attribute__((alias("...")))`
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notably missing but used in the code base:

  • __attribute__((always_inline)): This should be IMO a wrapper in compiler_hints.h. A compiler not supporting this will generate less efficient code (assuming this is actually sensibly used), but correct code, if this is ignored
  • __attribute__((align(N)))/attribute((aligned(N))): There is now <stdalign.h>that providesalignas()and the_Alignas()` since C11, so no need to use an extension for this.

Copy link
Copy Markdown
Contributor

@MrKevinWeiss MrKevinWeiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is useful information to have. You get 1 ACK from me :)

@maribu maribu added the Process: needs >1 ACK Integration Process: This PR requires more than one ACK label Jan 20, 2025
@github-actions github-actions bot added the Process: missing approvals Integration Process: PR needs more ACKS (handled by action) label Jan 20, 2025
Copy link
Copy Markdown
Contributor

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just two nits, otherwise another ACK from my side. Thanks for writing this up!

@github-actions github-actions bot removed the Process: missing approvals Integration Process: PR needs more ACKS (handled by action) label Jan 20, 2025
This adds reasoning for requiring standard compliant C code, and lists
generally accepted exceptions to this rule.

Co-authored-by: mguetschow <[email protected]>
@maribu maribu force-pushed the CODING_CONVENTIONS.md/std-c-exceptions branch from b437929 to efa622b Compare January 20, 2025 10:55
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs labels Jan 20, 2025
@riot-ci
Copy link
Copy Markdown

riot-ci commented Jan 20, 2025

Murdock results

✔️ PASSED

efa622b CODING_CONVENTIONS.md: Elaborate on C11 compliance

Success Failures Total Runtime
1 0 1 01m:29s

Artifacts

@MrKevinWeiss MrKevinWeiss added this pull request to the merge queue Jan 20, 2025
Merged via the queue into RIOT-OS:master with commit ea2818d Jan 21, 2025
@maribu maribu deleted the CODING_CONVENTIONS.md/std-c-exceptions branch January 21, 2025 09:20
@maribu
Copy link
Copy Markdown
Member Author

maribu commented Jan 21, 2025

Thx :)

@mguetschow mguetschow added this to the Release 2025.01 milestone Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: doc Area: Documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Process: needs >1 ACK Integration Process: This PR requires more than one ACK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants