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

[Mike Panetta]

Ok, I take back everything I said...  Its not any worse then
INTERRUPT(), or SIGNAL(), and with a little training even the newbies
should be able to understand that critical sections should be treated
with care. :)

Mike

On Wed, 2004-11-17 at 15:07, Mike Panetta wrote:
> 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
> 
> 
> _______________________________________________
> avr-gcc-list mailing list
> address@hidden
> http://www.avr1.org/mailman/listinfo/avr-gcc-list