Re: [avr-gcc-list] generic queue library for AVR GCC?

[David Brown]

> On Wed, 2004-11-17 at 04:43, David Brown wrote:
> [snip]
> >
> > Actually, you'd get the same problem if you put a return statement in
the
> > middle of the critical section.  I don't believe there is any safe way
to
> > guarentee that the critical section is exited properly if it is first
> > entered, without going beyond normal C.  Use a c++ object with a
constructor
> > that reads SREG and disables interrupts, and a destructor which restores
the
> > SREG would work.  In mspgcc, there is a new function attribute
"critical"
> > implemented which effectively wraps a function in the
save/disable/restore
> > code.
>
> Ok, maybe I completely missed the point, but why would anyone ever want
> to return in the middle of a critical section without restoring the
> SREG?  To me this (psudo) code makes no sense:
>
> enter_critical();
> do something
> if (something)
> return;
> do something else
> exit_critical();
>
> Thats bad coding.  It has an obvious bug in it.  Its like returning from

Of course it is bad coding - my point was that Richard's previous macro code
did *not* help in this situation (either in the sense of giving a
compilation error when you try it, or by sneaking in an extra
exit_critical() before the mid-function return).

> an error without deallocating memory that was allocated using malloc in
> some cases.  It should instead be:
>
> enter_critical();
> do something
> if (something)
> goto out;
> do something else
> out:
> exit_critical();
>
> Now before you flame me for use of goto, remember that every tool has
> its place, and IMNSHO this is one of them.  It does not make the code
> any harder to read, in fact it improves the readability by not hiding
> any functionality in some magical "you can't see it doing SREG stuff
> because its part of the function" junk.  You should be able to see what
> the code is doing by looking at it, and hiding what is very important
> information such as when the interrupts are disabled is a BAD idea to
> me.  It can lead to lazy or ignorant (of whats going on, not of how to
> code) programmers disabling interrupts for too long because they do not
> know they are disabled in the first place.
>

I fully agree that readability is paramount.  But I think a good name (like
"enter_critical") is clearer than manipulating SREG directly.

Personally, I'd like to see the addition of a "critical" function attribute,
like in mspgcc.  It is clearly readable, avoids any requirements for extra
local variables, generates optimal code, and generates any required "goto
out" automatically.  You can use it on inlined functions to avoid function
call overheads if you want.

> Personally I think you guys are overcomplicating what is a very simple
> and obvious issue of when (and how) to use a critical section...  But
> thats just my opinion...
>
>
> Mike
>
>