[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: dd SIGUSR1 race

From: Pádraig Brady
Subject: Re: dd SIGUSR1 race
Date: Sat, 27 Sep 2014 02:27:03 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2

On 09/26/2014 09:58 PM, Bernhard Voelker wrote:
> On 09/26/2014 05:05 PM, Pádraig Brady wrote:
>> Subject: [PATCH] dd: use more robust SIGUSR handling
> s/USR/USR1/
>> * src/dd.c (ifd_reopen): A new wrapper to ensure we
>> don't exit upon receiving a SIGUSR1 in a blocking open()
>> on a fifo for example.

I also added an EINTR wrapper for ftruncate().
Note I considered using SA_RESTART instead, but that
would mean that uncommitted read()/write() would block on SIGUSR1.
I could have undone the SA_RESTART before dd_copy(),
though that would be messier I think.

>> (iread): Process signals also after a short read.
>> (install_signal_handlers): Install SIGINFO/SIGUSR1 handler
>> even if set to SIG_IGN, as this is what the parent can easily
>> set from a shell script that can send SIGUSR1 without the
>> possiblity of inadvertently killing the dd process.
>> * doc/corutils.texi (dd invocation): Improve the example to
>> show robust usage wrt signal races and short reads.
>> * tests/dd/ A new test for various signal races.
>> * tests/ Reference the new test.
>> * NEWS: Mention the fix.
> Another minor nit:
> s/corutils/coreutils/
> The rest LGTM.
> What about adding "trap '' USR1;" to usage(), too?
> You know, many folks are only reading that instead of the
> texinfo manual.

Yes there should be no inconsistency here.

> OTOH, that stats-on-signal feature is such a detail that it may
> be worth removing from the manpage at all.

I didn't notice it in the man page.
I think it's best to leave that detail to the full info page,
so I've removed that.

Thanks for the review!

Latest version is attached.


Attachment: dd-siginfo.patch
Description: Text Data

reply via email to

[Prev in Thread] Current Thread [Next in Thread]