sys/event: add event_deferred_callback_post() helper
#21219
Conversation
0ab580f to
7f3e398
Compare
event_deferred_callback_start() helper
7f3e398 to
4bd0361
Compare
4bd0361 to
2ab559a
Compare
event_deferred_callback_start() helperevent_deferred_callback_post() helper
2ab559a to
b20f8cb
Compare
| * @brief Internal helper function for ztimer callback | ||
| * @param[in] arg event structure | ||
| */ | ||
| static inline void _event_deferred_post(void *arg) |
There was a problem hiding this comment.
Does inline do anything here?
There was a problem hiding this comment.
To my experience, modern compilers tread inline as white space and just do what makes most sense (e.g. not inlining when emitting a static function into the compilation unit is more compact, or inlinling even without inline if that results in better code).
I personally still tend to add those, as this can be a hint to someone reading the code.
My personal vision is that we just enable LTO by default though, and move all the static inline functions to non-static functions in C files. Not having any C code in the headers makes compatibility with C++ and rust a lot easier and LTO does a better job than static inlines anyway.
There was a problem hiding this comment.
Still, but since this is a local callback function it cannot be inlined anyway, right? So having inline as hint might rather confuse people.
There was a problem hiding this comment.
Yes, it cannot be inlined when a function ptr is set. Sorry, I missed that detail.
There was a problem hiding this comment.
gcc does not treat inline as white space instead "inline" removes the default/implicit extern so without adding another linkage maker the inline marked implementation is only available to be inlined (called in this compilation unit)
inline fn() { } << this can not be pointed to ( as it is no longer linkable) it is also not able to access static variables
inline static fn() {} << this can be inlined or staticly linked
inline extern fn() {} << this can be inlined or externaly linked
inline fn() {} << use this implementation is used if inlining
// another compilation unit may provide
extern fn() {} << use this implementation is used for extern or to be pointed to
https://godbolt.org/z/vb7YfKj3v
see compiler warning about no access to static and liker error about not linkable
what this is saying is inline is wrong as this function is not intended be directly called and it is not a good hint but one that leads the reader in the wrong direction
| * @param[in] callback callback to set up | ||
| * @param[in] arg callback argument to set up | ||
| */ | ||
| static inline void event_deferred_callback_post(event_deferred_callback_t *event, |
There was a problem hiding this comment.
I'd rather have this in a .c file, as I think not inlining this will generate smaller binaries.
Contribution description
Often there is a need to execute an event once after a delay.
This adds a convenience helper function for it.
Testing procedure
A test for the new function has been added to
tests/sys/event_periodic_callbackIssues/PRs references
cleaner alternative to #20861