Re: [avr-libc-dev] Disable/Restore all Interrupts

[Theodore A. Roth]

On Sun, 7 Sep 2003, Sam de Jongh Hepworth wrote:

> Hi
>
> Nice to know that you looked at my email. I noticed the { and } in the
> macros have raised some debate. Well the macros compile just fine - and the
> brackes have two functions:
>
> 1. It allows me to create a "temporary variable" to store the status of SREG
> 2. The compiler will complain if you forget RESTORE_INTERRUPTS();

The problem with this is that you have hidden syntax elements in
macros. True, the compiler will complain, but the complaint will not
not lead you to the cause of the complaint. If all the braces are
visible in the source code, you can use a good editor to find the
problem.

For example, I have emacs set up to auto indent my code. If I forget
the RESTORE_INTERRUPTS() macro, emacs will still indent the code
properly since the opening brace is hidden, gcc will fail to compile
the code and I'm left scratching my head about why it failed. Whereas,
if the mismatched braces are exposed to the editor, it starts
indenting things wrongly and I know I have bad code before I even try
to compile it.

Now, consider this happening to a C newbie, they'll never know what
hit them.

Consider this contrived code (a complete program):

  #include "macro.h"
  void foo (void)
  {
     OPEN_BRACE ();
  }

  int main (void)
  {
    foo ();

    return 0;
  }

And the error generated by gcc:

  $ make
  avr-gcc -g -Os -Wall -mmcu=atmega128     -c -o test.o test.c
  test.c: In function `foo':
  test.c:9: warning: `main' is normally a non-static function
  test.c:14: error: parse error at end of input
  make: *** [test.o] Error 1

It's not clear what the error is and would likely be less clear in a
larger program.

Now, I am just seeing that getting that compile failure eventually
forces you to fix the code before it even goes into the device. On the
other hand, not having the braces in the macros will let the code
compile which now has a bug (interrupts where not re-enabled) that
you have to track down which may be even harder and more time
consuming than fixing a compile error. That's one very good argument
for having the braces spread across separate macros.

Another possible down side of your macros is their use during a
context switch for a multitasking system. Hmmm, thinking about this
and looking at my context switching code, it doesn't look like a
problem if used in a sane way.

I can't really come up with a case where you would not use some form
of these macros within a simple block of code. I may be wrong about
this though.

Eric Weddington stated (see
http://mail.nongnu.org/archive/html/avr-libc-dev/2003-09/msg00003.html)
that some old version of pgmspace.h used braces in macros. I looked
for this and couldn't find the example. There are uses of braces in
macros in the avr-libc headers (for example, see avr/parity.h), but
those always open and close the brace within a single macro, which is
perfectly safe to do.

Also, Eric's note about C99 is moot since we already require gcc-3.3.x
for avr-libc and it is C99 compliant.

>
> Anyway I suspect may people use "sei" and "cli" in situations where a
> "nested" solution (like mine) would be much better. At least I did for a
> while...

Using sei and cli in a situation where you don't know the state of the
global interrupt flag is just wrong, as you have discovered.

I agree that we should supply some implementation of this in avr-libc
and document the usage. It's no a bad idea, we just need to hash out
the implementation.

Ted Roth