Refactor NotificationManager::Notification

Refactor NotificationManager::Notification to use constructors.
This reduces risk of coding errors (incl. buffer overflows) when
creating NotificationManager::Notification and copying text to it.

And fix a latent bug in ImmediateAlertService (include null terminator
in the bytes copied and properly set size) using the constructor.

Author: DavisNT

src/components/ble/AlertNotificationClient.cpp

@@ -156,10 +156,7 @@ void AlertNotificationClient::OnNotification(ble_gap_event* event) {
     size_t bufferSize = std::min(packetLen + stringTerminatorSize, maxBufferSize);
     auto messageSize = std::min(maxMessageSize, (bufferSize - headerSize));
 
-    NotificationManager::Notification notif;
-    os_mbuf_copydata(event->notify_rx.om, headerSize, messageSize - 1, notif.message.data());
-    notif.message[messageSize - 1] = '\0';
-    notif.size = messageSize;
+    NotificationManager::Notification notif(event->notify_rx.om, headerSize, messageSize);
     notif.category = Pinetime::Controllers::NotificationManager::Categories::SimpleAlert;
     notificationManager.Push(std::move(notif));
 

src/components/ble/AlertNotificationService.cpp

@@ -61,11 +61,8 @@ int AlertNotificationService::OnAlert(struct ble_gatt_access_ctxt* ctxt) {
     auto messageSize = std::min(maxMessageSize, (bufferSize - headerSize));
     Categories category;
 
-    NotificationManager::Notification notif;
-    os_mbuf_copydata(ctxt->om, headerSize, messageSize - 1, notif.message.data());
+    NotificationManager::Notification notif(ctxt->om, headerSize, messageSize);
     os_mbuf_copydata(ctxt->om, 0, 1, &category);
-    notif.message[messageSize - 1] = '\0';
-    notif.size = messageSize;
 
     // TODO convert all ANS categories to NotificationController categories
     switch (category) {

src/components/ble/DfuService.cpp

@@ -82,9 +82,7 @@ void DfuService::Init() {
 int DfuService::OnServiceData(uint16_t connectionHandle, uint16_t attributeHandle, ble_gatt_access_ctxt* context) {
 #ifndef PINETIME_IS_RECOVERY
   if (systemTask.GetSettings().GetDfuAndFsMode() == Pinetime::Controllers::Settings::DfuAndFsMode::Disabled) {
-    Pinetime::Controllers::NotificationManager::Notification notif;
-    memcpy(notif.message.data(), denyAlert, denyAlertLength);
-    notif.size = denyAlertLength;
+    Pinetime::Controllers::NotificationManager::Notification notif(denyAlert, denyAlertLength);
     notif.category = Pinetime::Controllers::NotificationManager::Categories::SimpleAlert;
     systemTask.GetNotificationManager().Push(std::move(notif));
     systemTask.PushMessage(Pinetime::System::Messages::OnNewNotification);

src/components/ble/FSService.cpp

@@ -53,9 +53,7 @@ void FSService::Init() {
 int FSService::OnFSServiceRequested(uint16_t connectionHandle, uint16_t attributeHandle, ble_gatt_access_ctxt* context) {
 #ifndef PINETIME_IS_RECOVERY
   if (systemTask.GetSettings().GetDfuAndFsMode() == Pinetime::Controllers::Settings::DfuAndFsMode::Disabled) {
-    Pinetime::Controllers::NotificationManager::Notification notif;
-    memcpy(notif.message.data(), denyAlert, denyAlertLength);
-    notif.size = denyAlertLength;
+    Pinetime::Controllers::NotificationManager::Notification notif(denyAlert, denyAlertLength);
     notif.category = Pinetime::Controllers::NotificationManager::Categories::SimpleAlert;
     systemTask.GetNotificationManager().Push(std::move(notif));
     systemTask.PushMessage(Pinetime::System::Messages::OnNewNotification);

src/components/ble/ImmediateAlertService.cpp

@@ -62,8 +62,7 @@ int ImmediateAlertService::OnAlertLevelChanged(uint16_t attributeHandle, ble_gat
       auto alertLevel = static_cast<Levels>(context->om->om_data[0]);
       auto* alertString = ToString(alertLevel);
 
-      NotificationManager::Notification notif;
-      std::memcpy(notif.message.data(), alertString, strlen(alertString));
+      NotificationManager::Notification notif(alertString, strlen(alertString) + 1);
       notif.category = Pinetime::Controllers::NotificationManager::Categories::SimpleAlert;
       notificationManager.Push(std::move(notif));
 

src/components/ble/NotificationManager.cpp

@@ -132,6 +132,23 @@ size_t NotificationManager::NbNotifications() const {
   return size;
 }
 
+NotificationManager::Notification::Notification() {
+}
+
+NotificationManager::Notification::Notification(const char* message, uint8_t size) {
+  uint8_t effectiveSize = std::min(std::max(size, (uint8_t) 1), NotificationManager::MessageSize);
+  memcpy(this->message.data(), message, effectiveSize - 1);
+  this->message[effectiveSize - 1] = '\0';
+  this->size = effectiveSize;
+}
+
+NotificationManager::Notification::Notification(const struct os_mbuf* om, int off, uint8_t size) {
+  uint8_t effectiveSize = std::min(std::max(size, (uint8_t) 1), NotificationManager::MessageSize);
+  os_mbuf_copydata(om, off, effectiveSize - 1, this->message.data());
+  this->message[effectiveSize - 1] = '\0';
+  this->size = effectiveSize;
+}
+
 const char* NotificationManager::Notification::Message() const {
   const char* itField = std::find(message.begin(), message.begin() + size - 1, '\0');
   if (itField != message.begin() + size - 1) {

src/components/ble/NotificationManager.h

@@ -5,6 +5,12 @@
 #include <cstddef>
 #include <cstdint>
 
+#define min // workaround: nimble's min/max macros conflict with libstdc++
+#define max
+#include <host/ble_gap.h>
+#undef max
+#undef min
+
 namespace Pinetime {
   namespace Controllers {
     class NotificationManager {
@@ -25,17 +31,23 @@ namespace Pinetime {
       static constexpr uint8_t MessageSize {100};
 
       struct Notification {
+      public:
         using Id = uint8_t;
         using Idx = uint8_t;
 
-        std::array<char, MessageSize + 1> message{};
-        uint8_t size;
         Categories category = Categories::Unknown;
         Id id = 0;
         bool valid = false;
 
+        Notification();
+        Notification(const char* message, uint8_t size);
+        Notification(const struct os_mbuf* om, int off, uint8_t size);
         const char* Message() const;
         const char* Title() const;
+
+      private:
+        std::array<char, MessageSize + 1> message {};
+        uint8_t size;
       };
 
       void Push(Notification&& notif);