Changeset 16a9ee1 in rtems for c

Bug report from Jiri Gaisler <jgais@…>:

I think I have found a bug in src/exec/scor/sparc/cpu/erc32.h in:

#define ERC32_Disable_interrupt( _source, _previous ) \

do { \

unsigned32 _level; \
unsigned32 _mask = 1 << (_source); \
\
sparc_disable_interrupts( _level ); \

(_previous) = ERC32_MEC.Interrupt_Mask; \
ERC32_MEC.Interrupt_Mask = _previous | _mask; \

sparc_enable_interrupts( _level ); \
(_previous) &= ~_mask; \ <- IS THIS CORRECT...?

} while (0)

The previous interrupt mask is returned after first clearing the
bit to be disabled, regardless whether the bit was set before or
not. If the bit was set (interrupt masked), subsequent call to
ERC32_Restore_interrupt() will enable the interrupt even though
it was supposed to be masked. This is indeed what happens in
DEBUG_puts when polled console I/O is used. In my opinion, the
last statement in the macro should be removed - what is your opinion?

I think the "~" shouldn't be there. I recall that the intent of that line
is to only return the state of the interrupts you were concerned with.
Removing the line returns entire state. Given that the value returned
shuold only be used in conjunction with the map, I suppose either removing
the ~ or the entire line is correct? I can go either way. Just let me
know which you think is more correct and the source will change. :)

Hmmm, just removing the '~' should be OK. DEBUG_puts() seems to be the
only user of ERC32_Restore_interrupt() anyway ...