Make HardwareSerial::begin() and end() interrupt safe and disable TX/RX if we can't allocate a buffer for them.

Prior to this change, the interrupt could fire during initialisation,
necessitating a deep check that the HardwareSerial structure had valid
_tx_buffer or _rx_buffer each time an interrupt occurred.

By keeping uart_t's and HardwareSerial's (txEnabled, _tx_buffer) and
(rxEnabled, _rx_buffer) in sync, we can remove this extra check, as
well as fixing a null pointer dereference if e.g. _tx_buffer allocation
failed and write() was subsequently called.

Author: alltheblinkythings

cores/esp8266/HardwareSerial.cpp

@@ -101,7 +101,8 @@ void uart_disarm_tx_interrupt(uart_t* uart);
 void uart_set_baudrate(uart_t* uart, int baud_rate);
 int uart_get_baudrate(uart_t* uart);
 
-uart_t* uart_init(int uart_nr, int baudrate, byte config);
+uart_t* uart_start_init(int uart_nr, int baudrate, byte config);
+void uart_finish_init(uart_t* uart);
 void uart_uninit(uart_t* uart);
 void uart_swap(uart_t* uart);
 
@@ -273,9 +274,8 @@ int uart_get_baudrate(uart_t* uart) {
     return uart->baud_rate;
 }
 
-uart_t* uart_init(int uart_nr, int baudrate, byte config, byte mode) {
+uart_t* uart_start_init(int uart_nr, int baudrate, byte config, byte mode) {
 
-    uint32_t conf1 = 0x00000000;
     uart_t* uart = (uart_t*) os_malloc(sizeof(uart_t));
 
     if(uart == 0) {
@@ -311,6 +311,12 @@ uart_t* uart_init(int uart_nr, int baudrate, byte config, byte mode) {
     uart_set_baudrate(uart, baudrate);
     USC0(uart->uart_nr) = config;
 
+    return uart;
+}
+
+void uart_finish_init(uart_t* uart) {
+    uint32_t conf1 = 0x00000000;
+
     uart_flush(uart);
     uart_interrupt_enable(uart);
 
@@ -323,8 +329,6 @@ uart_t* uart_init(int uart_nr, int baudrate, byte config, byte mode) {
     }
 
     USC1(uart->uart_nr) = conf1;
-
-    return uart;
 }
 
 void uart_uninit(uart_t* uart) {
@@ -485,31 +489,45 @@ HardwareSerial::HardwareSerial(int uart_nr) :
 }
 
 void HardwareSerial::begin(unsigned long baud, byte config, byte mode) {
+    InterruptLock il;
 
     // disable debug for this interface
     if(uart_get_debug() == _uart_nr) {
         uart_set_debug(UART_NO);
     }
 
-    _uart = uart_init(_uart_nr, baud, config, mode);
+    if (_uart) {
+        os_free(_uart);
+    }
+    _uart = uart_start_init(_uart_nr, baud, config, mode);
 
     if(_uart == 0) {
         return;
     }
 
-    if(_uart->rxEnabled) {
-        if(!_rx_buffer)
-            _rx_buffer = new cbuf(SERIAL_RX_BUFFER_SIZE);
+    // Disable the RX and/or TX functions if we fail to allocate circular buffers.
+    // The user can confirm they are enabled with isRxEnabled() and isTxEnabled().
+    if(_uart->rxEnabled && !_rx_buffer) {
+        _rx_buffer = new cbuf(SERIAL_RX_BUFFER_SIZE);
+        if(!_rx_buffer) {
+            _uart->rxEnabled = false;
+        }
     }
-    if(_uart->txEnabled) {
-        if(!_tx_buffer)
-            _tx_buffer = new cbuf(SERIAL_TX_BUFFER_SIZE);
+    if(_uart->txEnabled && !_tx_buffer) {
+        _tx_buffer = new cbuf(SERIAL_TX_BUFFER_SIZE);
+        if(!_tx_buffer) {
+            _uart->txEnabled = false;
+        }
     }
     _written = false;
     delay(1);
+
+    uart_finish_init(_uart);
 }
 
 void HardwareSerial::end() {
+    InterruptLock il;
+
     if(uart_get_debug() == _uart_nr) {
         uart_set_debug(UART_NO);
     }
@@ -660,16 +678,10 @@ HardwareSerial::operator bool() const {
 }
 
 void ICACHE_RAM_ATTR HardwareSerial::_rx_complete_irq(char c) {
-    if(_rx_buffer) {
-        _rx_buffer->write(c);
-    }
+    _rx_buffer->write(c);
 }
 
 void ICACHE_RAM_ATTR HardwareSerial::_tx_empty_irq(void) {
-    if(_uart == 0)
-        return;
-    if(_tx_buffer == 0)
-        return;
     const int uart_nr = _uart->uart_nr;
     size_t queued = _tx_buffer->getSize();
     if(!queued) {