[avr-gcc-list] Re: issues with struct and volatile

[David Brown]

klaas wrote:
> Try this:
>
> struct
> {
>   uint8_t *tx_head;
>   uint8_t volatile *tx_tail;
>   uint8_t tx_buffer[DBG_BUF_SZ];
>
>   /* blah blah */
>   uint8_t dbg_timer;
> } dbg_serial_port;
>
>
> Declare the pointer volatile, not the value it is pointing to

The principle is right, the details are wrong.  You want:

        uint8_t * volatile tx_tail;

These things are so very easy to get wrong that I always use typedefs:

        typedef uint8_t * pUint8_t;
        volatile pUint8_t tx_tail;

That way there is no room for confusion or error.

And I presume it is only tx_tail that could be changing in interrupts, 
otherwise tx_head must also be declared volatile.


As a general point, it is a bad idea to use a 16-bit value as the 
"volatile" variable modified in the interrupt routine and checked by the 
main code.  The problem is that you cannot access the whole 16 bits 
atomically (without added interrupt disables or other code).  It doesn't 
matter for a simple check, but if the main code can change the value of 
tx_tail, or needs to make use of the value (other than for a simple 
comparison), while the interrupt is enabled, then there's going to be 
trouble.  It would be much better to store tx_tail (and tx_head) as an 
8-bit index into tx_buffer - 8-bit data can be handled atomically.

mvh.,

David



> Pink Boy wrote:
> > Hi,
> >
> > I recently run into an issue with a struct with some volatile members 
> > and was wondering is this is a problem with me or gcc, ar avr-gcc.
> >
> > I have a structure used to define a buffer for a serial port somewhat 
> > like
> > this.
> >
> > struct
> > {
> >    uint8_t *tx_head;
> >    volatile uint8_t *tx_tail;
> >    uint8_t tx_buffer[DBG_BUF_SZ];
> >
> >    /* blah blah */
> >    uint8_t dbg_timer;
> > } dbg_serial_port;
> >
> > In my main loop I have a code that waits to see if the buffer is full.
> >
> >      while(dbg_serial_port.tx_head == dbg_serial_port.txtail)
> >           ;
> >
> > And of course with compiler flags set to optimize for size -Os it hangs
> > and waits forever.
> >
> > The only way to get it to work is to declare the whole structure and 
> > tx_tail as volatile.  I'd be just annoyed, but if I do that I get a whole
> > bunch of compiler warnings on stuff like
> >
> > uint8_t *temp = dbg_serial_port.tx_head;
> >
> > I can fix those with a cast but doing that is like going after a fly 
> > with a hammer.
> >
> > Is this a bug with gcc's handling of volatile or just me?
> >
> > PS: Using the WinAvr 20090313.
> >
> > Matt
> >
> >
> > _______________________________________________
> > AVR-GCC-list mailing list
> > address@hidden
> > http://lists.nongnu.org/mailman/listinfo/avr-gcc-list
> >
> >
> >