Re: [avr-gcc-list] volatile...

[Dave Hansen]

From: Paulo Marques <address@hidden>

> Graham Davies wrote:
> > "David Brown" wrote:
> > > You are missing a number of points ...
> >
> > Well, I think we're getting close to complete coverage now!
>
> Well, since we are going for complete coverage, I'll add my 2 cents, then.

You've opened some new cans of worms here, but I'll only make one small 
comment

[...]

> static uint16_t atomic_read_16(uint16_t *ptr)
> {
>         uint16_t ret;
>
>         cli();
>         ret = (volatile)(*ptr);

Your cast to volatile here is not only unnecessary, it's wrong.  You are 
actually casting the uint16_t intermediate value to signed int, which is 
then converted (back) to uint16_t.  If you tried to model an atomic_read_32 
on this function, you might not like the results.

On newer conmpilers (with C99 compliance) it shouldn't even compile (implied 
int no longer allowed).

>         sei();

OK, a second comment:  I like to save SREG before the CLI, then restore SREG 
rather than blindly enabling interupts with SEI.  That way, if I happen to 
call the function with interrupts disabled, they aren't unintentionally 
enabled early.

>         return ret;
> }

Regards,

  -=Dave

_________________________________________________________________
WIN up to $10,000 in cash or prizes – enter the Microsoft Office Live 
Sweepstakes http://clk.atdmt.com/MRT/go/aub0050001581mrt/direct/01/