sys/util.h: Add IS_ALIGNED macro #67243
Conversation
zephyrbot
requested review from
andyross,
ceolin,
dcpleung,
nashif and
peter-mitsis
|
The following west manifest projects have been modified in this Pull Request:
Note: This message is automatically posted and updated by the Manifest GitHub Action. |
zephyrbot
added
manifest
manifest-sof
DNM
submanifests/optional.yaml
Outdated
| @@ -34,7 +34,7 @@ manifest: | |||
| groups: | |||
| - optional | |||
| - name: sof | |||
| revision: e7cb489d430dc2181e4a5f7f953ed1eaeec6668d | |||
| revision: pull/35/head | |||
There was a problem hiding this comment.
This isn't the right way to do this. The fix needs to merge first, otherwise these YAML files become constant games of ping-pong chasing upstream patches.
There was a problem hiding this comment.
This is just to have the fix tested with Zephyr's CI before it's merged in the external module.
There was a problem hiding this comment.
The SOF macro IS_ALIGN() is used in only 4 places! Just rename it to SOF_ALIGN() as requested by https://docs.zephyrproject.org/latest/contribute/coding_guidelines/index.html#rule-a-3-macro-name-collisions and make your forward, backward and backflip manifest life much easier.
EDIT: rejected in thesofproject/sof#8755
include/zephyr/sys/util.h
Outdated
| @@ -277,6 +277,13 @@ extern "C" { | |||
| */ | |||
| #define CONCAT(x, y) _DO_CONCAT(x, y) | |||
|
|
|||
| #ifndef IS_ALIGNED | |||
There was a problem hiding this comment.
Don't do "opportunistic" macro definitions like this because debugging macros is the worst nightmare ever.
Macros with commonly used names such as MIN, MAX, ARRAY_SIZE, must NOT be modified or protected to avoid name collisions with other implementations. In particular, they must not be prefixed to place them in a Zephyr-specific namespace, re-defined using #undef, or conditionally excluded from compilation using #ifndef.
This is the simplest, cheapest and safest to avoid the collision with the incoming Zephyr macro IS_ALIGN() submitted in zephyrproject-rtos/zephyr#67243 As SOF depends on Zephyr, most SOF symbols should be prefixed SOF_ or something specific. This is compliant with: https://docs.zephyrproject.org/latest/contribute/coding_guidelines/index.html#rule-a-3-macro-name-collisions Signed-off-by: Marc Herbert <[email protected]>
yonsch
dismissed stale reviews from ceolin, andyross, and peter-mitsis
via
b6e3d3d
zephyrbot
removed
manifest
manifest-sof
DNM
|
Wait: does the intermediate state with only one commit compiles? Without warning? I'm all for smaller commits and smaller PRs but isn't this one step too far? Could hinder |
Added IS_ALIGNED macro to check if a pointer is aligned to a given alignment. Additionally, removed a macro with a conflicting name in drivers/crypto_intel. Signed-off-by: Yonatan Schachter <[email protected]>
|
Setting the milestone as 3.7 as we are past rc1 in 3.6. If you disagree with this please correct the milestone. |
Add an IS_ALIGNED macro, to test if a value
xis aligned to an alignmentalign. This has been independently defined in multiple areas throughout the tree and modules. This PR fixes only those instances where the macro name collides with the sys/util macro.