-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Use extra char in notification message array for null terminator #1695
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
base: main
Are you sure you want to change the base?
Use extra char in notification message array for null terminator #1695
Conversation
Also move category reading down for clarity
|
Build size and comparison to main:
|
FintasticMan
left a comment
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.
Another solution to this would be to not make the message array 1 char bigger, which would make it more obvious when a size/len does or doesn't include the null terminator.
|
I think the terminator will always cause confusion as long as the truncation needs to happen outside of NotificationManager. I think NotificationManager should accept any std::string and truncate it internally. |
|
I think that would be a shame personally, because I think it should be possible to make it clear what is what through good naming and consistency. For example by using len to indicate number of chars excluding null terminator, and size being number of bytes total. Anyway, the reason I would rather not use std::string is because it is always allocated on the heap, which seems wasteful if we're just putting it onto the stack again right after. |
|
Would string_view be better? |
…niTimeOrg#1695 commit d7ca78b Author: Finlay Davidson <finlay.davidson@coderclass.nl> Date: Fri Mar 17 13:48:26 2023 +0100 alertnotificationservice: Make use of extra char in message array for \0 Also move category reading down for clarity commit d1b16a7 Author: Finlay Davidson <finlay.davidson@coderclass.nl> Date: Fri Mar 17 13:38:52 2023 +0100 alertnotificationclient: Make use of extra char in message array for \0
…niTimeOrg#1695 commit d7ca78b Author: Finlay Davidson <finlay.davidson@coderclass.nl> Date: Fri Mar 17 13:48:26 2023 +0100 alertnotificationservice: Make use of extra char in message array for \0 Also move category reading down for clarity commit d1b16a7 Author: Finlay Davidson <finlay.davidson@coderclass.nl> Date: Fri Mar 17 13:38:52 2023 +0100 alertnotificationclient: Make use of extra char in message array for \0
The message array in the Notification struct has a size of MaximumMessageSize + 1, for a trailing null terminator if the message is MaximumMessageSize chars. This extra char isn't used for notifications coming from bluetooth, meaning that notifications are truncated 1 character earlier if they are too long. This change is minor and just means that notifications can be 1 character longer.