Hi Tim,
Tim Kientzle wrote:
> * Since the signal handler just flags for future
> printing, why not just install it unconditionally at
> the top of the program? I can't see where it accomplishes
> anything to install/uninstall the signal handlers as you've
> done. Since the signal handler is one line (and installing
> it is two lines), I would suggest just putting it into
> bsdtar.c.
I considered that, but figured that it was better to keep or
restore the old signal handler at any point when we're not
going to do anything in response to the signal -- of course
it's not an issue for SIGINFO since that signal is discarded
by default, but I didn't see "we can save a few lines of code
by not cleaning up" as a compelling argument.
> * It seems you could actually eliminate siginfo.c by just
> storing a fully-formatted string in the bsdtar structure.
> I think your siginfo_setinfo can be replaced with this:
>
> snprintf(bsdtar->siginfo_message, sizeof(bsdtar->siginfo_message),
> "appropriate format", args... );
>
> and siginfo_printinfo with this simple stanza:
>
> if (bsdtar->siginfo_received) {
> bsdtar->siginfo_received = 0;
> fprintf(stderr, "%s\n", bsdtar->siginfo_message);
> }
Well... not quite. In siginfo_printinfo the following things
are also done:
1. Printing "\n" before or after the string depending on whether
bsdtar->verbose is set, in order to "play nice" with the lines
which are mandated behaviour of "tar -v".
2. Printing (X / Y) with appropriate X and Y, or not (if we don't
have a file size).
In the original version of this code (in tarsnap), I didn't print
how far we had gotten through the current file, and at that point
I had all the code manually inlined -- but I split it off into
siginfo.c when I found myself throwing around 4 copies of 15 lines
of code in write.c. Even if inlining the printing into write.c
and read.c saved a few lines of code, I think the approach of using
a separate siginfo.c is worthwhile for the added cleanliness alone.
Colin Percival
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
