Solar Designer
2015-09-12 20:19:32 UTC
Sayantan -
Your use of a signal handler in bt.c is buggy:
The signal_stop variable should be "volatile int" (like we use in
signals.c), or we might want to convert all of them to "volatile
sig_atomic_t". The "volatile" keyword is required - without it,
compiler optimization may break your code from working any time (I've
seen it happen in similar cases). The compiler may simply pre-load
signal_stop into a register in create_tables() and then it would not
detect this variable changing inside the loop.
Another bug is that stdio is not async signal safe. You shouldn't use
stdio streams in signal handlers. You also shouldn't use printf family
functions in signal handlers (these functions are supposed to be
thread-safe when you invoke them on buffers not shared with any other
thread, but they might not be async signal safe). I suggest that you
only set the variable in the signal handler, and postpone the fprintf()
until the program notices that signal_stop is set. Alternatively, you
may use write().
When you're detecting whether signal_stop is set, you currently do:
if (signal_stop) {
signal_stop = 0;
alarm(0);
Depending on how your timer is initialized (one-shot vs. multiple), this
might contain a race condition: what if the timer fires a second time
after you've reset "signal_stop = 0;" but before alarm(0); completes?
I think you want to swap these two lines: do alarm(0); first, and reset
signal_stop to 0 next. That way, you'd ignore a possible second signal.
Alexander
Your use of a signal handler in bt.c is buggy:
The signal_stop variable should be "volatile int" (like we use in
signals.c), or we might want to convert all of them to "volatile
sig_atomic_t". The "volatile" keyword is required - without it,
compiler optimization may break your code from working any time (I've
seen it happen in similar cases). The compiler may simply pre-load
signal_stop into a register in create_tables() and then it would not
detect this variable changing inside the loop.
Another bug is that stdio is not async signal safe. You shouldn't use
stdio streams in signal handlers. You also shouldn't use printf family
functions in signal handlers (these functions are supposed to be
thread-safe when you invoke them on buffers not shared with any other
thread, but they might not be async signal safe). I suggest that you
only set the variable in the signal handler, and postpone the fprintf()
until the program notices that signal_stop is set. Alternatively, you
may use write().
When you're detecting whether signal_stop is set, you currently do:
if (signal_stop) {
signal_stop = 0;
alarm(0);
Depending on how your timer is initialized (one-shot vs. multiple), this
might contain a race condition: what if the timer fires a second time
after you've reset "signal_stop = 0;" but before alarm(0); completes?
I think you want to swap these two lines: do alarm(0); first, and reset
signal_stop to 0 next. That way, you'd ignore a possible second signal.
Alexander