Read/write buffers in HardwareSerial should be declared volatile

[nickgammon]

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.

[fish2000]

@nickgammon agreed.

[matthijskooijman]

I wondered if volatile is really needed when a variable is only written to from the main loop and only read from ISR context (or vice versa), but thinking on it a bit more that would indeed still require volatile. So, +1 from me.

[DrDiettrich]

A volatile buffer will improve nothing, because

• the items are bytes with atomic access,
• waiting for changing data does not make sense
• and is delegated to comparing head and tail indices before a buffer access.
  I.e. head and tail are updated only after a new byte has been inserted or removed from the buffer so that valid data is accessible all the time.

Also volatile applies only to RX head and TX tail which are updated in the ISR. The complementary TX head and RX tail indices are only updated outside ISR and their use in the ISR is restricted to == (equal) compare, no problem when it fails (false negative) due to update in progress. The ISR won't wait until the main thread will have finished updating, the main thread instead will continue polling until success.