arduino / arduino/ArduinoCore-avr

Read/write buffers in HardwareSerial should be declared volatile

Open
#178 4 comments 0 reactions 1 assignee Claimed by @cmaglie View on GitHub
Dominant language
C
Stars
1.5k
Forks
1.1k
PR merge metrics
No merged PRs in 30d

Description

The buffers used by HardwareSerial meet the conditions for volatile variables:
- They are read/written to inside an ISR
- They are accessed outside an ISR

Therefore in HardwareSerial.cpp (IDE 1.0.6) the declaration should read:

```
struct ring_buffer
{
volatile unsigned char buffer[SERIAL_BUFFER_SIZE];
volatile unsigned int head;
volatile unsigned int tail;
};
```

And in HardwareSerial.h (IDE 1.5.8) it should read:

```
volatile unsigned char _rx_buffer[SERIAL_RX_BUFFER_SIZE];
volatile unsigned char _tx_buffer[SERIAL_TX_BUFFER_SIZE];
```

These are private members, and access to the data is only through Serial.read(), Serial.write() etc., and thus examples and existing code should continue to work.

Admittedly those buffers appear to work OK despite not being volatile, but this would be luck (the compiler not being able to optimize accesses to an array) rather than good management.

---

The same remarks would apply to SoftwareSerial.

---

Preliminary testing appears to show that this does not affect code size. This therefore is really documentation, and future-proofing. Also, since Arduino users may be reading the supplied libraries as examples of "good practice" it sets a good example to make those buffers volatile.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.