-
Notifications
You must be signed in to change notification settings - Fork 2k
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.
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
gcoap: add helper function to get request header from a request memo #18095
gcoap: add helper function to get request header from a request memo #18095
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK. Please squash. The linting errors are actually unrelated.
a733843
to
584681f
Compare
* @return The request header for the given request memo. | ||
*/ | ||
static inline coap_hdr_t *gcoap_request_memo_get_hdr(const gcoap_request_memo_t *memo) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not critical: what's your opinion on adding an assert(memo != NULL)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would require to include assert.h
and also, since the macro uses RIOT_FILE_RELATIVE
the provided line would be wrong (it would point to the including C-file). That's why I intentially omitted it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the premise that the compiler really inlines this inline function ..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it will still take the line and file from code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But RIOT_FILE_RELATIVE
is broken wtr, not the compiler ;-).
584681f
to
e9a12b7
Compare
Rebased, after #17801 got merged. |
e9a12b7
to
cbbde07
Compare
Found another occurance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still looks good. ACK!
Contribution description
Just some code-deduplication.
Testing procedure
examples/gcoap
should still compile with and without thegcoap_forward_proxy
.Issues/PRs references
Spotted by @cgundogan in #17801 but can be merged independent of it (but might cause merge conflicts there or here).