Date: Tue, 20 May 2008 20:42:38 -0700 From: Tim Kientzle <kientzle@freebsd.org> To: Colin Percival <cperciva@freebsd.org> Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: cvs commit: src/usr.bin/tar Makefile bsdtar.h read.c siginfo.c write.c Message-ID: <48339A2E.1060805@freebsd.org> In-Reply-To: <200805180624.m4I6Ol12077124@repoman.freebsd.org> References: <200805180624.m4I6Ol12077124@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
After looking at this a bit, I'm wondering if it
couldn't be made rather a bit simpler:
* 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.
* 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);
}
This would also allow you to vary the informational formatting
depending on the operation. Sure, these stanzas would be
duplicated in a couple of places, but I think this would be
significantly less total code in the end.
Colin Percival wrote:
> cperciva 2008-05-18 06:24:47 UTC
>
> FreeBSD src repository
>
> Modified files:
> usr.bin/tar Makefile bsdtar.h read.c write.c
> Added files:
> usr.bin/tar siginfo.c
> Log:
> Add SIGINFO (and for portability to SIGINFO-lacking systems, SIGUSR1)
> handling to bsdtar. When writing archives (including copying via the
> @archive directive) a line is output to stderr indicating what is being
> done (adding or copying), the path, and how far through the file we are;
> extracting currently does not report progress within each file, but
> this is likely to happen eventually.
>
> Discussed with: kientzle
> Obtained from: tarsnap
>
> Revision Changes Path
> 1.35 +1 -1 src/usr.bin/tar/Makefile
> 1.31 +6 -0 src/usr.bin/tar/bsdtar.h
> 1.37 +26 -0 src/usr.bin/tar/read.c
> 1.1 +147 -0 src/usr.bin/tar/siginfo.c (new)
> 1.67 +25 -0 src/usr.bin/tar/write.c
>
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?48339A2E.1060805>