ezieragabriel / arduino

Automatically exported from code.google.com/p/arduino
Other
0 stars 0 forks source link

use uint8_t with the serial ringbuffer #1078

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
In HardwareSerial.cpp the ring buffer is defined as

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

The head and tail variables should be redefined as uint8_t since the value is 
always between 0 and 64 (inclusive). This saves 2 bytes of SRAM per buffer.

Also, the current code looks like it has a subtle bug. Reading head and tail 
from outside the ISR is not atomic since they are 16 bit, yet the code does not 
disable interrupts to ensure atomicity. In general that could result in reading 
a value that the variable never actually had. 

It turns out though that in this case the high byte will always be 0, and there 
is no hidden bug. Nevertheless some people have been confused by this[0]. 
Changing to uint8_t eliminates this confusion, while also saving space.

Footnote:
[0] For an example of people thinking there may be a bug here, see 
http://bleaklow.com/2012/02/29/why_im_ditching_the_arduino_software_platform.htm

Original issue reported on code.google.com by KevinCat...@gmail.com on 21 Oct 2012 at 3:57

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Fixed here:
https://github.com/cmaglie/Arduino/commit/e0a9a7676b7b3b0db359b4edcabaf3ef9946ea
e7

Original comment by c.mag...@arduino.cc on 30 Jul 2013 at 9:39