Bluetooth: Host: Disable dangling pointer warning in le_adv_recv #48690
Conversation
This commit temporarily disables the dangling pointer warning (`-Wdangling-pointer`) in the `le_adv_recv` function in order to allow this file to build successfully with the GCC 12, which introduced this warning. A proper fix will be provided later as part of the GitHub issue zephyrproject-rtos#48459. Signed-off-by: Stephanos Ioannidis <[email protected]>
stephanosio
requested review from
alwa-nordic,
jhedberg,
Vudentz and
sjanc
as code owners
| #pragma GCC diagnostic ignored "-Wpragmas" | ||
| #pragma GCC diagnostic ignored "-Wdangling-pointer" | ||
| #endif | ||
|
|
||
| info->addr = &id_addr; |
There was a problem hiding this comment.
I may have missed the discussion leading to this particular fix, but why don't we just set the pointer to NULL after the SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&scan_cbs, ...) loop on line 474? (and move the above line to right before that loop, since it seems we're setting the pointer unnecessarily early here).
There was a problem hiding this comment.
Update: I read the referenced issue, and still think that the setting to NULL is a better initial fix (i.e. "option a") and then work on the allocation/lifetime as a follow up.
There was a problem hiding this comment.
I may have missed the discussion leading to this particular fix
Please see #48459 (comment).
This is not intended to be a "fix," but only a temporary workaround to get it building on the upcoming Zephyr SDK 0.15.0 release with GCC 12.
I did not implement the "option a" because I wanted to keep this PR as less intrusive as possible so as to not introduce any regressions (i.e. are we completely sure that info->addr is not referenced by the caller and setting it to NULL would not result in any regressions)?
If you can provide a proper fix (either implementing "option a" or "option b") before the Zephyr SDK 0.15.0 is mainlined (likely in one or two weeks), then this PR can be closed.
There was a problem hiding this comment.
I did not implement the "option a" because I wanted to keep this PR as less intrusive as possible so as to not introduce any regressions (i.e. are we completely sure that info->addr is not referenced by the caller and setting it to NULL would not result in any regressions)?
Perceived intrusiveness is a subjective thing. The preprocessor stuff you add in this PR appears more intrusive to me than a one line var = NULL; addition. I don't quite get what you mean by your regression comment: if the calling function tried to access the the pointer it would be accessing invalid memory since the stack frame of the function that set it is no longer valid. Setting the pointer to NULL is therefore a better initial fix IMO.
There was a problem hiding this comment.
@jhedberg Can you or one of the Bluetooth collaborators send a patch for that in the meantime then? I would prefer not to introduce any behavioural changes to this subsystem myself since I am not familiar enough with it.
|
#48727 supersedes this. |
This commit temporarily disables the dangling pointer warning
(
-Wdangling-pointer) in thele_adv_recvfunction in order to allowthis file to build successfully with the GCC 12, which introduced this
warning.
A proper fix will be provided later as part of the GitHub issue
#48459.
Signed-off-by: Stephanos Ioannidis [email protected]