Skip to content

Commit f69ab9e

Browse files
committed
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.
1 parent 8d7a04e commit f69ab9e

7 files changed

Lines changed: 49 additions & 19 deletions

src/components/ble/AlertNotificationClient.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,7 @@ void AlertNotificationClient::OnNotification(ble_gap_event* event) {
156156
size_t bufferSize = std::min(packetLen + stringTerminatorSize, maxBufferSize);
157157
auto messageSize = std::min(maxMessageSize, (bufferSize - headerSize));
158158

159-
NotificationManager::Notification notif;
160-
os_mbuf_copydata(event->notify_rx.om, headerSize, messageSize - 1, notif.message.data());
161-
notif.message[messageSize - 1] = '\0';
162-
notif.size = messageSize;
159+
NotificationManager::Notification notif(event->notify_rx.om, headerSize, messageSize);
163160
notif.category = Pinetime::Controllers::NotificationManager::Categories::SimpleAlert;
164161
notificationManager.Push(std::move(notif));
165162

src/components/ble/AlertNotificationService.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,8 @@ int AlertNotificationService::OnAlert(struct ble_gatt_access_ctxt* ctxt) {
6161
auto messageSize = std::min(maxMessageSize, (bufferSize - headerSize));
6262
Categories category;
6363

64-
NotificationManager::Notification notif;
65-
os_mbuf_copydata(ctxt->om, headerSize, messageSize - 1, notif.message.data());
64+
NotificationManager::Notification notif(ctxt->om, headerSize, messageSize);
6665
os_mbuf_copydata(ctxt->om, 0, 1, &category);
67-
notif.message[messageSize - 1] = '\0';
68-
notif.size = messageSize;
6966

7067
// TODO convert all ANS categories to NotificationController categories
7168
switch (category) {

src/components/ble/DfuService.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,7 @@ void DfuService::Init() {
8282
int DfuService::OnServiceData(uint16_t connectionHandle, uint16_t attributeHandle, ble_gatt_access_ctxt* context) {
8383
#ifndef PINETIME_IS_RECOVERY
8484
if (systemTask.GetSettings().GetDfuAndFsMode() == Pinetime::Controllers::Settings::DfuAndFsMode::Disabled) {
85-
Pinetime::Controllers::NotificationManager::Notification notif;
86-
memcpy(notif.message.data(), denyAlert, denyAlertLength);
87-
notif.size = denyAlertLength;
85+
Pinetime::Controllers::NotificationManager::Notification notif(denyAlert, denyAlertLength);
8886
notif.category = Pinetime::Controllers::NotificationManager::Categories::SimpleAlert;
8987
systemTask.GetNotificationManager().Push(std::move(notif));
9088
systemTask.PushMessage(Pinetime::System::Messages::OnNewNotification);

src/components/ble/FSService.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,7 @@ void FSService::Init() {
5353
int FSService::OnFSServiceRequested(uint16_t connectionHandle, uint16_t attributeHandle, ble_gatt_access_ctxt* context) {
5454
#ifndef PINETIME_IS_RECOVERY
5555
if (systemTask.GetSettings().GetDfuAndFsMode() == Pinetime::Controllers::Settings::DfuAndFsMode::Disabled) {
56-
Pinetime::Controllers::NotificationManager::Notification notif;
57-
memcpy(notif.message.data(), denyAlert, denyAlertLength);
58-
notif.size = denyAlertLength;
56+
Pinetime::Controllers::NotificationManager::Notification notif(denyAlert, denyAlertLength);
5957
notif.category = Pinetime::Controllers::NotificationManager::Categories::SimpleAlert;
6058
systemTask.GetNotificationManager().Push(std::move(notif));
6159
systemTask.PushMessage(Pinetime::System::Messages::OnNewNotification);

src/components/ble/ImmediateAlertService.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,7 @@ int ImmediateAlertService::OnAlertLevelChanged(uint16_t attributeHandle, ble_gat
6262
auto alertLevel = static_cast<Levels>(context->om->om_data[0]);
6363
auto* alertString = ToString(alertLevel);
6464

65-
NotificationManager::Notification notif;
66-
std::memcpy(notif.message.data(), alertString, strlen(alertString) + 1);
67-
notif.size = strlen(alertString) + 1;
65+
NotificationManager::Notification notif(alertString, strlen(alertString) + 1);
6866
notif.category = Pinetime::Controllers::NotificationManager::Categories::SimpleAlert;
6967
notificationManager.Push(std::move(notif));
7068

src/components/ble/NotificationManager.cpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,36 @@ size_t NotificationManager::NbNotifications() const {
132132
return size;
133133
}
134134

135+
NotificationManager::Notification::Notification() {
136+
}
137+
138+
/**
139+
* Make a Notification from const char* buffer.
140+
*
141+
* @param message the const char* buffer with the message
142+
* @param size the size of complete message, including the final 0x00 byte
143+
*/
144+
NotificationManager::Notification::Notification(const char* message, uint8_t size) {
145+
uint8_t effectiveSize = std::min(std::max(size, static_cast<uint8_t>(1)), NotificationManager::MessageSize);
146+
memcpy(this->message.data(), message, effectiveSize - 1);
147+
this->message[effectiveSize - 1] = '\0';
148+
this->size = effectiveSize;
149+
}
150+
151+
/**
152+
* Make a Notification from a value in const struct os_mbuf* buffer.
153+
*
154+
* @param om the const struct os_mbuf* buffer containing the message
155+
* @param off the offset of the message - passed to os_mbuf_copydata()
156+
* @param size the size of complete message, including the final 0x00 byte
157+
*/
158+
NotificationManager::Notification::Notification(const struct os_mbuf* om, int off, uint8_t size) {
159+
uint8_t effectiveSize = std::min(std::max(size, static_cast<uint8_t>(1)), NotificationManager::MessageSize);
160+
os_mbuf_copydata(om, off, effectiveSize - 1, this->message.data());
161+
this->message[effectiveSize - 1] = '\0';
162+
this->size = effectiveSize;
163+
}
164+
135165
const char* NotificationManager::Notification::Message() const {
136166
const char* itField = std::find(message.begin(), message.begin() + size - 1, '\0');
137167
if (itField != message.begin() + size - 1) {

src/components/ble/NotificationManager.h

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@
55
#include <cstddef>
66
#include <cstdint>
77

8+
#define min // workaround: nimble's min/max macros conflict with libstdc++
9+
#define max
10+
#include <host/ble_gap.h>
11+
#undef max
12+
#undef min
13+
814
namespace Pinetime {
915
namespace Controllers {
1016
class NotificationManager {
@@ -25,17 +31,23 @@ namespace Pinetime {
2531
static constexpr uint8_t MessageSize {100};
2632

2733
struct Notification {
34+
public:
2835
using Id = uint8_t;
2936
using Idx = uint8_t;
3037

31-
std::array<char, MessageSize + 1> message{};
32-
uint8_t size;
3338
Categories category = Categories::Unknown;
3439
Id id = 0;
3540
bool valid = false;
3641

42+
Notification();
43+
Notification(const char* message, uint8_t size);
44+
Notification(const struct os_mbuf* om, int off, uint8_t size);
3745
const char* Message() const;
3846
const char* Title() const;
47+
48+
private:
49+
std::array<char, MessageSize + 1> message {};
50+
uint8_t size;
3951
};
4052

4153
void Push(Notification&& notif);

0 commit comments

Comments
 (0)