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

[Mike Panetta]

On Wed, 2004-11-17 at 12:30, E. Weddington wrote:
> David Brown wrote:
> 
> >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.
> >  
> >
> I think that's a *great* idea!
> 
> Personally, I don't care for all the methods that have been proposed on 
> this thread that "hide" the manipulation of SREG (various macros, etc.), 
> because of the very reason that they can be too easily be the causes of 
> some bad behaviour unless someone is vigilant about using them. I 
> generally prefer to be explicit about it.
> 
> *But*, if there is a reasonable way to have the compiler enforce good 
> behaviour, such as the "critical" function attribute proposed above, 
> then I think that beats out all other methods.

ACK!  NOO!!  Its bad bad bad.  It hides the fact that the function is
critical so well that it makes it many times easier to make a simple
mistake that would throw interrupt latency (or worse) all to hell, and
the programmer may not even know why!

Ill give you 2 ways that this can be innocently broken.  way number 1,
allocate extra variables on the stack (or malloc) in the 'critical
function'.  An end user (programmer) that may not be the one that
origionally designed the function may add code that allocates space on
the stack or mallocs space inside said critical function.  This will
cause interrupts to be disabled far too long.  Example code:

functionheader.h :
        void criticalfunction(void) __attribute__ ((critical));

functionbody.c :
        void criticalfunction(void) //notice you do not see the attribute here,
a comment could get around this however
        {
                int a, b, c; //I assume the compiler would allocate this space 
BEFORE
disabling interrupts
                a = somefunc();
                b = someotherfunc();
                if (b)
                {
                        int d; //This would be allocated with interrupts 
disabled!!! 
Interrupt latency increases.
                        d = somefunc();
                        c = d + b;
                }
        }

Another example, in this example someone else has picked up the code (or
it may even be the same person that wrote it) and is trying to debug a
problem:

functionbody.c :
        void criticalfunction(void)
        {
                int a, b, c;

                a = somefunc();
                b = somefunc2();
                c = a - b;
                if (a < 0) //this is an error case that should not happen and 
is being
debugged
                {
                        printf("Why is a %d?\n", a);
                }

        }

Now one of 2 things could happen, either the interrupt latency is shot
to hell and the code stops working, or the code locks up because some
function that printf() is calling requires interrupts to be enabled. 
Either way we are screwed.  If functions like enter_critical(), and
exit_critical() were used, it would be obvious where the printf should
go, and the code could even be manipulated to make it safe to put it
where it would be needed if need be.  Having 'critical functions'
defeats this.

I admit that both of the examples above are very contrived, but it
should be obvious why this is bad when one actually gets to writing a
critical piece of code.  It makes debugging difficult for sure, and that
is not a good thing at all.

My $3.50 ;)

Mike

> 
> My $0.02.
> 
> Eric
> 
> _______________________________________________
> avr-gcc-list mailing list
> address@hidden
> http://www.avr1.org/mailman/listinfo/avr-gcc-list