InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.64k stars 907 forks source link

Use extra char in notification message array for null terminator #1695

Open FintasticMan opened 1 year ago

FintasticMan commented 1 year ago

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.

github-actions[bot] commented 1 year ago
Build size and comparison to main: Section Size Difference
text 406604B -32B
data 940B 0B
bss 53568B 0B
Riksu9000 commented 1 year ago

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.

FintasticMan commented 1 year ago

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.

Riksu9000 commented 1 year ago

Would string_view be better?