Re: [wdiff-bugs] [bug #36546] error executing diff results in undeleted temporary files

[Martin von Gagern]

Hi François,

thanks a lot for your input.

On 27.05.2012 20:07, François Pinard wrote:
>> When the diff process spawned by wdiff dies for some reason, wdiff
>> will exit immediately, without unlinking the temporary files
>> containing the input to diff.
> 
> Hi, Martin.  It was a long time ago, but presumably, it could have been
> on purpose to keep files around, in view of debugging (for easier retry).

At least the exit call which I identified as the reason the testsuite
left some temporary files was one I added rather recently for
wdiff-1.0.0, and at least there it wasn't intentional. All those calls
to the error() function might be a different matter, though.

>> Perhaps the cleaner solution would be replacing all that longjmp stuff
>> with some cleanup function to be registered using atexit.
> 
> I vaguely remember that "all that longjmp stuff" much displeased me even
> if I wrote it.  it was the only "sane" avenue I was able to figure out
> for an interruption.  According to what I read at the time, one cannot
> do much within an interrupt, setting a flag is OK however.  This forces
> wdiff to check the flag in loops and a bit everywhere, and this is ugly.
> I had the feeling of busy-waiting, if you understand what I'm trying to
> express.  I may be at fault of course, but that day, I should confess
> that C programming did not much impress me. :-).

I'm not too certain as to what things one may safely (and portably) do
inside a signal handler myself. I know there is even a sig_atomic_t
which might be better suited as the type of the flag. And I guess the
flag should be volatile as well, to avoid the optimizer from caching its
value in some way. Guess a fix to the issue at hand will include changes
here as well.

I guess this "busy waiting" isn't as bad as it feels, because the
application actually is busy in that situation, so it does more than
just wait. My current intent is to keep that part, at least for the time
being. I still intend to have some interrupt flag, and check for it
regularly. But when the flag is set, I'd rather call exit() instead of
longjmp(), and have the cleanup handler registered with atexit() take
care of any remaining work.

The libc man page actually states that "functions registered using
atexit() are not called  if  a process terminates abnormally because of
the delivery of a signal" so I guess changing from longjmp to atexit
will not help to get rid of that flag-based approach. The only
alternative would be calling the cleanup function in the signal handler
itself. Calling unlink from a signal handler should be safe enough, from
what I read in the libc documentation, although again I'm not sure about
portability. So probably it's best to keep things as they are for now,
with respect to the flag-based approach.

Greetings,
 Martin

signature.asc
Description: OpenPGP digital signature