Bluetooth: TBS: Fixed return call_index 0 after overflow #64545
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LuckeTech
requested review from
jhedberg,
Vudentz,
Thalley and
sjanc
as code owners
zephyrbot
requested review from
asbjornsabo,
Casper-Bonde-Bose,
fredrikdanebjer,
kruithofa,
larsgk and
MariuszSkamra
LuckeTech
force-pushed
the
fix_tbs_call_index_overflow
branch
2 times, most recently
from
9a23c83 to
5ebf829
Compare
subsys/bluetooth/audio/tbs.c
Outdated
Comment on lines
313
to
324
| if (call == NULL) { | ||
| return next_call_index++; | ||
| } | ||
|
|
||
| next_call_index++; | ||
| if (next_call_index == UINT8_MAX) { | ||
| /* call_index = 0 reserved for outgoing calls */ | ||
| next_call_index = 1; | ||
| } | ||
|
|
||
| /* For each new call, the call index should be incremented */ | ||
| next_call_index++; | ||
|
|
||
| if (call == NULL) { | ||
| /* call == NULL is true for (next_call_index - 1)*/ | ||
| return (next_call_index - 1); | ||
| } |
There was a problem hiding this comment.
Either I'm missing something, or this won't return 255
There was a problem hiding this comment.
Yes, you are right.
There was a problem hiding this comment.
Should be fixed now. @MariuszSkamra
LuckeTech
force-pushed
the
fix_tbs_call_index_overflow
branch
3 times, most recently
from
3a5cf6c to
be03570
Compare
subsys/bluetooth/audio/tbs.c
Outdated
| } | ||
|
|
||
| const struct bt_tbs_call *call = lookup_call(next_call_index); |
There was a problem hiding this comment.
Please do not declare the variables in the middle of the scope
LuckeTech
force-pushed
the
fix_tbs_call_index_overflow
branch
2 times, most recently
from
229dc8d to
42fb1ae
Compare
Thalley
requested changes
Oct 30, 2023
There was a problem hiding this comment.
Nice catch on the bug, although it is pretty mood since CONFIG_BT_TBS_MAX_CALLS can maximum be 16 at this point:
config BT_TBS_MAX_CALLS
int "Telephone Bearer Service Maximum Number Of Calls Supported"
default 3
range 1 16
help
Sets the maximum number of calls the service supports per bearer.So next_call_index could never be larger than 17.
In any case, we might as well fix it.
|
The commit message should also be elaborated a bit more and should state what is being fixed and why |
LuckeTech
force-pushed
the
fix_tbs_call_index_overflow
branch
2 times, most recently
from
5b2642b to
42be018
Compare
Thalley
previously approved these changes
Oct 30, 2023
LuckeTech
force-pushed
the
fix_tbs_call_index_overflow
branch
from
42be018 to
18ab507
Compare
Thalley
approved these changes
Oct 30, 2023
Fixed range of call_index 1 to 255, since 0 is reserved for outgoing calls. Signed-off-by: Benjamin Lucke <[email protected]>
LuckeTech
force-pushed
the
fix_tbs_call_index_overflow
branch
from
9eb84c8 to
429756e
Compare
MariuszSkamra
approved these changes
Nov 3, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Function returned 0 after an overflow, moved wrap before return.
Return next_call_index = 0, since 0 is reserved.