Skip to content

Commit 5d2394c

Browse files
committed
Improve memory safety of SerialBuffer
1 parent c8b9dc2 commit 5d2394c

File tree

1 file changed

+21
-15
lines changed

1 file changed

+21
-15
lines changed

src/serial/SerialInputHandler.cpp

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ using namespace std::string_view_literals;
5959

6060
using namespace OpenShock;
6161

62-
const int64_t PASTE_INTERVAL_THRESHOLD_MS = 20;
63-
const std::size_t SERIAL_BUFFER_CLEAR_THRESHOLD = 512;
62+
const int64_t PASTE_INTERVAL_THRESHOLD_MS = 20;
63+
const std::size_t SERIAL_BUFFER_MAX_CAPACITY = 4096;
6464

6565
static bool s_echoEnabled = true;
6666
static std::vector<OpenShock::Serial::CommandGroup> s_commandGroups;
@@ -290,7 +290,7 @@ class SerialBuffer {
290290
DISABLE_MOVE(SerialBuffer);
291291

292292
public:
293-
constexpr SerialBuffer()
293+
SerialBuffer()
294294
: m_data(nullptr)
295295
, m_size(0)
296296
, m_capacity(0)
@@ -304,12 +304,12 @@ class SerialBuffer {
304304
}
305305
inline ~SerialBuffer() { delete[] m_data; }
306306

307-
constexpr char* data() { return m_data; }
308-
constexpr std::size_t size() const { return m_size; }
309-
constexpr std::size_t capacity() const { return m_capacity; }
310-
constexpr bool empty() const { return m_size == 0; }
307+
constexpr const char* data() const noexcept { return m_data == nullptr ? "" : m_data; }
308+
constexpr std::size_t size() const noexcept { return m_size; }
309+
constexpr std::size_t capacity() const noexcept { return m_capacity; }
310+
constexpr bool empty() const noexcept { return m_size == 0; }
311311

312-
constexpr void clear() { m_size = 0; }
312+
constexpr void clear() noexcept { m_size = 0; }
313313
inline void destroy()
314314
{
315315
delete[] m_data;
@@ -322,6 +322,11 @@ class SerialBuffer {
322322
{
323323
size = (size + 31) & ~31; // Align to 32 bytes
324324

325+
if (size > SERIAL_BUFFER_MAX_CAPACITY) {
326+
OS_LOGE(TAG, "Refused to reserve %zu bytes, clearing buffer", size);
327+
size = SERIAL_BUFFER_MAX_CAPACITY;
328+
m_size = 0;
329+
}
325330
if (size <= m_capacity) {
326331
return;
327332
}
@@ -332,27 +337,28 @@ class SerialBuffer {
332337
delete[] m_data;
333338
}
334339

335-
m_data = newData;
336-
m_capacity = size;
340+
m_data = newData;
341+
m_capacity = size;
342+
m_data[m_capacity - 1] = 0;
337343
}
338344

339345
inline void push_back(char c)
340346
{
341347
if (m_size >= m_capacity) {
342-
reserve(m_capacity + 16);
348+
reserve(m_size + 16);
343349
}
344350

345351
m_data[m_size++] = c;
346352
}
347353

348-
constexpr void pop_back()
354+
constexpr void pop_back() noexcept
349355
{
350356
if (m_size > 0) {
351357
--m_size;
352358
}
353359
}
354360

355-
constexpr operator std::string_view() const { return std::string_view(m_data, m_size); }
361+
constexpr operator std::string_view() const { return std::string_view(data(), m_size); }
356362

357363
private:
358364
char* m_data;
@@ -621,7 +627,7 @@ void _serialRxTask(void*)
621627
_processSerialLine(buffer);
622628

623629
// Deallocate memory if the buffer is too large
624-
if (buffer.capacity() > SERIAL_BUFFER_CLEAR_THRESHOLD) {
630+
if (buffer.capacity() > SERIAL_BUFFER_MAX_CAPACITY) {
625631
buffer.destroy();
626632
} else {
627633
buffer.clear();
@@ -647,7 +653,7 @@ void _serialRxTask(void*)
647653
_processSerialLine(buffer);
648654

649655
// Deallocate memory if the buffer is too large
650-
if (buffer.capacity() > SERIAL_BUFFER_CLEAR_THRESHOLD) {
656+
if (buffer.capacity() > SERIAL_BUFFER_MAX_CAPACITY) {
651657
buffer.destroy();
652658
} else {
653659
buffer.clear();

0 commit comments

Comments
 (0)