|
From: | Paulo Marques |
Subject: | Re: [avr-gcc-list] volatile... |
Date: | Fri, 15 Dec 2006 15:58:26 +0000 |
User-agent: | Thunderbird 1.5.0.7 (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; 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).
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.
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.
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."
[Prev in Thread] | Current Thread | [Next in Thread] |