[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Paulo Marques
Subject: Re: [avr-gcc-list] volatile...
Date: Fri, 15 Dec 2006 15:58:26 +0000
User-agent: Thunderbird (X11/20060909)

Dave Hansen wrote:
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

I was afraid of that (the cans of worms, not your comment) ;)


static uint16_t atomic_read_16(uint16_t *ptr)
    uint16_t ret;

    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).

I warned that I didn't try to compile the code, so these mistakes (forgetting the uint16_t in the cast to volatile) were likely to happen :)

But you're actually wrong about the volatile not being needed. Because the "sei" instruction doesn't claim anything about memory clobbers, without volatile the compiler would be free to re-order instructions and do the "sei" before the assignment.

This is no theoretical scenario. Just search the archives for previous threads over this.


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.

I mentioned that too, but in my example the atomic_read should only be used _outside_ interrupts (inside interrupts you could just access the variable the normal way), so no harm should come from using cli/sei instead of store flags / restore flags.

But I agree that a generic "atomic.h" implementation should save flags / restore flags instead of cli/sei to be really safe to use anywhere.

Paulo Marques - www.grupopie.com

"The face of a child can say it all, especially the
mouth part of the face."

reply via email to

[Prev in Thread] Current Thread [Next in Thread]