Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

build_system/xfa: change API to fix alignment #20960

Merged
merged 2 commits into from
Nov 7, 2024
Merged

build_system/xfa: change API to fix alignment #20960

merged 2 commits into from
Nov 7, 2024

Conversation

maribu
Copy link
Member

@maribu maribu commented Nov 6, 2024

Contribution description

This changes the API of xfa from

XFA(array_name, prio) type element_name = INITIALIZER;

to

XFA(type, array_name, prio) element_name = INITIALIZER;

this allows forcing natural alignment of the type, fixing failing tests
on native64.

Testing procedure

The tests from #20959 is included here. It should now pass.

Issues/PRs references

Includes #20959

@maribu
tests/build_system/xfa: improve test coverage
2839224 
This increases the test coverage for XFA:

- Multiple entries are added from a single file
- Priorities are assigned to enforce a given order
- The size of the `struct` to add is changed to no longer be a power of
  2
@github-actions github-actions bot added Area: network Area: Networking Area: tests Area: tests and testing framework Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: drivers Area: Device drivers Area: OTA Area: Over-the-air updates Area: sys Area: System Area: examples Area: Example Applications labels Nov 6, 2024
@maribu maribu added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: core Area: RIOT kernel. Handle PRs marked with this with care! Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed Area: network Area: Networking Area: tests Area: tests and testing framework Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: drivers Area: Device drivers Area: OTA Area: Over-the-air updates Area: sys Area: System Area: examples Area: Example Applications labels Nov 6, 2024
This was referenced Nov 6, 2024
Merged
Merged
@riot-ci
Copy link

riot-ci commented Nov 6, 2024 •
edited
Loading

Murdock results

✔️ PASSED

2b6f65a build_system/xfa: change API to fix alignment

Success Failures Total Runtime
10214 0 10215 14m:37s

Artifacts

@maribu maribu force-pushed the build_system/xfa/fix-alignment branch from 347cf07 to 0f7b399 Compare November 6, 2024 20:20
@github-actions github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: network Area: Networking Area: tests Area: tests and testing framework Area: drivers Area: Device drivers Area: OTA Area: Over-the-air updates Platform: ESP Platform: This PR/issue effects ESP-based platforms Area: cpu Area: CPU/MCU ports Area: sys Area: System Area: examples Area: Example Applications labels Nov 6, 2024
mguetschow
mguetschow requested changes Nov 7, 2024
View reviewed changes
Copy link
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.

Looks good, just a minor nit below.

drivers/include/sdmmc/sdmmc.h Outdated Show resolved Hide resolved
@maribu
build_system/xfa: change API to fix alignment
2b6f65a 
This changes the API of xfa from

    XFA(array_name, prio) type element_name = INITIALIZER;

to

    XFA(type, array_name, prio) element_name = INITIALIZER;

this allows forcing natural alignment of the type, fixing failing tests
on `native64`.
@maribu maribu force-pushed the build_system/xfa/fix-alignment branch from 0f7b399 to 2b6f65a Compare November 7, 2024 15:30
mguetschow
mguetschow approved these changes Nov 7, 2024
View reviewed changes
Copy link
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.

LGTM then, thank you!

@benpicco benpicco added this pull request to the merge queue Nov 7, 2024
Merged via the queue into RIOT-OS:master with commit d65dec6 Nov 7, 2024
27 checks passed
@maribu
Copy link
Member Author

maribu commented Nov 7, 2024

Thx a lot!

@maribu maribu deleted the build_system/xfa/fix-alignment branch November 7, 2024 19:47
@maribu maribu mentioned this pull request Dec 15, 2024
Merged
@maribu
Copy link
Member Author

maribu commented Dec 15, 2024

Backport provided in #21088

@maribu maribu added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Dec 15, 2024
@MrKevinWeiss MrKevinWeiss added this to the Release 2025.01 milestone Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mguetschow mguetschow mguetschow approved these changes

@miri64 miri64 Awaiting requested review from miri64 miri64 is a code owner

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@kaspar030 kaspar030 Awaiting requested review from kaspar030 kaspar030 is a code owner

@bergzand bergzand Awaiting requested review from bergzand bergzand is a code owner

@benpicco benpicco Awaiting requested review from benpicco

@Teufelchen1 Teufelchen1 Awaiting requested review from Teufelchen1

@vincent-d vincent-d Awaiting requested review from vincent-d vincent-d is a code owner

@gschorcht gschorcht Awaiting requested review from gschorcht gschorcht is a code owner

Assignees
No one assigned
Labels
Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: examples Area: Example Applications Area: network Area: Networking Area: OTA Area: Over-the-air updates Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Platform: ESP Platform: This PR/issue effects ESP-based platforms Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
Release 2025.01
Development

Successfully merging this pull request may close these issues.
5 participants
@maribu @riot-ci @mguetschow @benpicco @MrKevinWeiss