[request-sponsor] Adding status support to dd

2007-04-05 Thread casper....@sun.com

>Note that SA_RESTART must be set; otherwise dd from a pipe or other slow
>device will return an error condition.  I also like Casper's suggestion 
>to use sprintf and write to stderr, since sprintf is now
>async-signal-safe.

Note also that there are three gettext()s of constant strings;
these should probably be cached at the beginning of the program
so that we completely avoid any unsafe pitfalls.

const char *instr = gettext("%llu+%llu records in\n";
const char *outstr = gettext("%llu+%llu records out\n";
const char *truncstr = gettext("%llu truncated record(s)\n";

Casper



[request-sponsor] Adding status support to dd

2007-04-05 Thread Bart Smaalders
Bonnie Corwin wrote:
> Hi Bart,
> 
> I assume your comment about helping to get this into shape means you 
> will sponsor Ryan's work.  Can someone file a bug, if there isn't one 
> already, so I can add this to the table?
> 
> Thanks.
> 
> Bonnie


6543165 dd should display incremental progress ala linux in response to 
sigusr1

- Bart


-- 
Bart Smaalders  Solaris Kernel Performance
barts at cyber.eng.sun.com  http://blogs.sun.com/barts



[request-sponsor] Adding status support to dd

2007-04-05 Thread Bonnie Corwin
Hi Bart,

I assume your comment about helping to get this into shape means you 
will sponsor Ryan's work.  Can someone file a bug, if there isn't one 
already, so I can add this to the table?

Thanks.

Bonnie

Bart Smaalders wrote:
> Matty wrote:
> 
>> On 4/5/07, James Carlson  wrote:
>>
>>> Darren J Moffat writes:
>>> > Matty wrote:
>>> > > Howdy,
>>> > >
>>> > > Most Linux and BSD distributions ship with a version of dd that
>>> > > displays the status of a copy operation when a SIGUSR1 signal is
>>> > > received. For some reason /usr/bin/dd on Solaris hosts doesn't 
>>> contain
>>> > > this capability, and I would like to add it. After reading 
>>> through the
>>> > > source code for dd.c, it looks like adding this support would be as
>>> > > simple as installing a signal handler for SIGUSR1, and having it 
>>> call
>>> > > the function stats. I am attaching code (which I copied from the dd
>>> > > that ships with Fedora Core Linux)
>>> >
>>> > What license is that dd code you copied under ?
>>> >
>>> > You can't just copy code like that without considering the licenses.
>>>
>>> Indeed.  That's GPLv2 code, which would require OpenSolaris dd to
>>> become GPLv2 as well.
>>>
>>> That can't be done.  Either start provably from scratch or use
>>> software under a compatible license.
>>
>>
>> Your absolutely right, and Stephen Hahn was nice enough to point this
>> out in an email last night. Since the existing code uses signal to
>> register the SIGINT signal handler (I reckon the issues Bart mentioned
>> are minimized since it exit()'s after printing the statistics):
>>
>> if (signal(SIGINT, SIG_IGN) != SIG_IGN)
>> {
>>   (void) signal(SIGINT, term);
>> }
>>
>> Do folks see any issue with using similar code to add a SIGUSR1 signal 
>> handler?:
>>
>> if (signal(SIGUSR1, SIG_IGN) != SIG_IGN)
>> {
>>(void) signal(SIGUSR1, stats);
>> }
>>
>> This would minmize the amount of code that would need to be added to
>> get stats functionality, and would address the license issues. If this
>> solution isn't acceptable, could I get a sponsor assigned to help me
>> work through a community approved solution?
>>
>> Thanks,
>> - Ryan
> 
> 
> Note that SA_RESTART must be set; otherwise dd from a pipe or other slow
> device will return an error condition.  I also like Casper's suggestion 
> to use sprintf and write to stderr, since sprintf is now
> async-signal-safe.
> 
> I can help you get this into shape.
> 
> - Bart
> 
> 
> 



[request-sponsor] Adding status support to dd

2007-04-05 Thread Darren J Moffat
Matty wrote:
> Howdy,
> 
> Most Linux and BSD distributions ship with a version of dd that
> displays the status of a copy operation when a SIGUSR1 signal is
> received. For some reason /usr/bin/dd on Solaris hosts doesn't contain
> this capability, and I would like to add it. After reading through the
> source code for dd.c, it looks like adding this support would be as
> simple as installing a signal handler for SIGUSR1, and having it call
> the function stats. I am attaching code (which I copied from the dd
> that ships with Fedora Core Linux)

What license is that dd code you copied under ?

You can't just copy code like that without considering the licenses.

-- 
Darren J Moffat



[request-sponsor] Adding status support to dd

2007-04-05 Thread Matty
On 4/5/07, Matty  wrote:
> On 4/5/07, James Carlson  wrote:
> > Darren J Moffat writes:
> > > Matty wrote:
> > > > Howdy,
> > > >
> > > > Most Linux and BSD distributions ship with a version of dd that
> > > > displays the status of a copy operation when a SIGUSR1 signal is
> > > > received. For some reason /usr/bin/dd on Solaris hosts doesn't contain
> > > > this capability, and I would like to add it. After reading through the
> > > > source code for dd.c, it looks like adding this support would be as
> > > > simple as installing a signal handler for SIGUSR1, and having it call
> > > > the function stats. I am attaching code (which I copied from the dd
> > > > that ships with Fedora Core Linux)
> > >
> > > What license is that dd code you copied under ?
> > >
> > > You can't just copy code like that without considering the licenses.
> >
> > Indeed.  That's GPLv2 code, which would require OpenSolaris dd to
> > become GPLv2 as well.
> >
> > That can't be done.  Either start provably from scratch or use
> > software under a compatible license.
>
> Your absolutely right, and Stephen Hahn was nice enough to point this
> out in an email last night. Since the existing code uses signal to
> register the SIGINT signal handler (I reckon the issues Bart mentioned
> are minimized since it exit()'s after printing the statistics):
>
> if (signal(SIGINT, SIG_IGN) != SIG_IGN)
> {
>(void) signal(SIGINT, term);
> }
>
> Do folks see any issue with using similar code to add a SIGUSR1 signal 
> handler?:
>
> if (signal(SIGUSR1, SIG_IGN) != SIG_IGN)
> {
> (void) signal(SIGUSR1, stats);
> }
>
> This would minmize the amount of code that would need to be added to
> get stats functionality, and would address the license issues. If this
> solution isn't acceptable, could I get a sponsor assigned to help me
> work through a community approved solution?

Ooops -- I pasted the wrong code in the second snippet. The signal()
call would actually register a function that reregisters the signal
handler, and then calls stats. Of the two signal solutions proposed in
this thread (sigaction and signal), which would folks recommend using?

Sorry for the typo,
- Ryan
-- 
UNIX Administrator
http://prefetch.net



[request-sponsor] Adding status support to dd

2007-04-05 Thread casper....@sun.com

>This is prob. overkill in this situation... since the main loop
>will only do stdio just before exiting, it could set
>signal handlers to SIG_IGN before doing any IO which should
>prevent any problems from arising.



Why not use snprintf and write?

Casper




[request-sponsor] Adding status support to dd

2007-04-05 Thread Matty
On 4/5/07, James Carlson  wrote:
> Darren J Moffat writes:
> > Matty wrote:
> > > Howdy,
> > >
> > > Most Linux and BSD distributions ship with a version of dd that
> > > displays the status of a copy operation when a SIGUSR1 signal is
> > > received. For some reason /usr/bin/dd on Solaris hosts doesn't contain
> > > this capability, and I would like to add it. After reading through the
> > > source code for dd.c, it looks like adding this support would be as
> > > simple as installing a signal handler for SIGUSR1, and having it call
> > > the function stats. I am attaching code (which I copied from the dd
> > > that ships with Fedora Core Linux)
> >
> > What license is that dd code you copied under ?
> >
> > You can't just copy code like that without considering the licenses.
>
> Indeed.  That's GPLv2 code, which would require OpenSolaris dd to
> become GPLv2 as well.
>
> That can't be done.  Either start provably from scratch or use
> software under a compatible license.

Your absolutely right, and Stephen Hahn was nice enough to point this
out in an email last night. Since the existing code uses signal to
register the SIGINT signal handler (I reckon the issues Bart mentioned
are minimized since it exit()'s after printing the statistics):

if (signal(SIGINT, SIG_IGN) != SIG_IGN)
{
   (void) signal(SIGINT, term);
}

Do folks see any issue with using similar code to add a SIGUSR1 signal handler?:

if (signal(SIGUSR1, SIG_IGN) != SIG_IGN)
{
(void) signal(SIGUSR1, stats);
}

This would minmize the amount of code that would need to be added to
get stats functionality, and would address the license issues. If this
solution isn't acceptable, could I get a sponsor assigned to help me
work through a community approved solution?

Thanks,
- Ryan
-- 
UNIX Administrator
http://prefetch.net



[request-sponsor] Adding status support to dd

2007-04-05 Thread James Carlson
Darren J Moffat writes:
> Matty wrote:
> > Howdy,
> > 
> > Most Linux and BSD distributions ship with a version of dd that
> > displays the status of a copy operation when a SIGUSR1 signal is
> > received. For some reason /usr/bin/dd on Solaris hosts doesn't contain
> > this capability, and I would like to add it. After reading through the
> > source code for dd.c, it looks like adding this support would be as
> > simple as installing a signal handler for SIGUSR1, and having it call
> > the function stats. I am attaching code (which I copied from the dd
> > that ships with Fedora Core Linux)
> 
> What license is that dd code you copied under ?
> 
> You can't just copy code like that without considering the licenses.

Indeed.  That's GPLv2 code, which would require OpenSolaris dd to
become GPLv2 as well.

That can't be done.  Either start provably from scratch or use
software under a compatible license.

-- 
James Carlson, Solaris Networking  
Sun Microsystems / 1 Network Drive 71.232W   Vox +1 781 442 2084
MS UBUR02-212 / Burlington MA 01803-2757   42.496N   Fax +1 781 442 1677



[request-sponsor] Adding status support to dd

2007-04-05 Thread Bart Smaalders
Matty wrote:
> On 4/5/07, James Carlson  wrote:
>> Darren J Moffat writes:
>> > Matty wrote:
>> > > Howdy,
>> > >
>> > > Most Linux and BSD distributions ship with a version of dd that
>> > > displays the status of a copy operation when a SIGUSR1 signal is
>> > > received. For some reason /usr/bin/dd on Solaris hosts doesn't 
>> contain
>> > > this capability, and I would like to add it. After reading through 
>> the
>> > > source code for dd.c, it looks like adding this support would be as
>> > > simple as installing a signal handler for SIGUSR1, and having it call
>> > > the function stats. I am attaching code (which I copied from the dd
>> > > that ships with Fedora Core Linux)
>> >
>> > What license is that dd code you copied under ?
>> >
>> > You can't just copy code like that without considering the licenses.
>>
>> Indeed.  That's GPLv2 code, which would require OpenSolaris dd to
>> become GPLv2 as well.
>>
>> That can't be done.  Either start provably from scratch or use
>> software under a compatible license.
> 
> Your absolutely right, and Stephen Hahn was nice enough to point this
> out in an email last night. Since the existing code uses signal to
> register the SIGINT signal handler (I reckon the issues Bart mentioned
> are minimized since it exit()'s after printing the statistics):
> 
> if (signal(SIGINT, SIG_IGN) != SIG_IGN)
> {
>   (void) signal(SIGINT, term);
> }
> 
> Do folks see any issue with using similar code to add a SIGUSR1 signal 
> handler?:
> 
> if (signal(SIGUSR1, SIG_IGN) != SIG_IGN)
> {
>(void) signal(SIGUSR1, stats);
> }
> 
> This would minmize the amount of code that would need to be added to
> get stats functionality, and would address the license issues. If this
> solution isn't acceptable, could I get a sponsor assigned to help me
> work through a community approved solution?
> 
> Thanks,
> - Ryan

Note that SA_RESTART must be set; otherwise dd from a pipe or other slow
device will return an error condition.  I also like Casper's suggestion 
to use sprintf and write to stderr, since sprintf is now
async-signal-safe.

I can help you get this into shape.

- Bart



-- 
Bart Smaalders  Solaris Kernel Performance
barts at cyber.eng.sun.com  http://blogs.sun.com/barts



[request-sponsor] Adding status support to dd

2007-04-05 Thread William Kucharski
Matty wrote:

> Do folks see any issue with using similar code to add a SIGUSR1 signal 
> handler?:
> 
> if (signal(SIGUSR1, SIG_IGN) != SIG_IGN)
> {
>(void) signal(SIGUSR1, stats);
> }
> 
> This would minmize the amount of code that would need to be added to
> get stats functionality, and would address the license issues. If this
> solution isn't acceptable, could I get a sponsor assigned to help me
> work through a community approved solution?

Did you read Bart Smaalders' email regarding stdio not being async signal safe? 
  While the window is fairly small for SIGINT, since term() will exit() right 
after calling stats(), the only reason to add SIGUSR1 support is with the idea 
that it may be called repeatedly, widening the window in which a deadlock could 
theoretically occur.

To be safe you should really do something like use sigaction(2) and have it 
install a signal mask that blocks all other signals while stats() is running, 
something like:

 usr_handler.sa_handler = stats;
 usr_handler.sa_flags = SA_RESTART;
 (void) sigfillset(&usr_handler.sa_mask);

 if (sigaction(SIGUSR1, &usr_handler, NULL) != 0) {
 /* crud, sigaction(2) failed, do something useful here */
 }

 [ ... ]

 William Kucharski
 william.kucharski at sun.com