From owner-cvs-src@FreeBSD.ORG Thu May 22 20:55:31 2008 Return-Path: Delivered-To: cvs-src@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id F022A10656C8 for ; Thu, 22 May 2008 20:55:31 +0000 (UTC) (envelope-from cperciva@freebsd.org) Received: from pd3mo1so.prod.shaw.ca (idcmail-mo1so.shaw.ca [24.71.223.10]) by mx1.freebsd.org (Postfix) with ESMTP id C1FFD8FC34 for ; Thu, 22 May 2008 20:55:31 +0000 (UTC) (envelope-from cperciva@freebsd.org) Received: from pd2mr3so.prod.shaw.ca (pd2mr3so-qfe3.prod.shaw.ca [10.0.141.108]) by l-daemon (Sun ONE Messaging Server 6.0 HotFix 1.01 (built Mar 15 2004)) with ESMTP id <0K1A006BSFFNUM90@l-daemon> for cvs-src@freebsd.org; Thu, 22 May 2008 14:54:59 -0600 (MDT) Received: from pn2ml10so.prod.shaw.ca ([10.0.121.80]) by pd2mr3so.prod.shaw.ca (Sun Java System Messaging Server 6.2-7.05 (built Sep 5 2006)) with ESMTP id <0K1A001BIFFM3GG0@pd2mr3so.prod.shaw.ca> for cvs-src@freebsd.org; Thu, 22 May 2008 14:54:59 -0600 (MDT) Received: from hexahedron.daemonology.net ([24.80.10.198]) by l-daemon (Sun ONE Messaging Server 6.0 HotFix 1.01 (built Mar 15 2004)) with SMTP id <0K1A00LJ7FFKXI60@l-daemon> for cvs-src@freebsd.org; Thu, 22 May 2008 14:54:57 -0600 (MDT) Received: (qmail 1076 invoked from network); Thu, 22 May 2008 20:54:55 +0000 Received: from unknown (HELO hexahedron.daemonology.net) (127.0.0.1) by localhost with SMTP; Thu, 22 May 2008 20:54:55 +0000 Date: Thu, 22 May 2008 13:54:54 -0700 From: Colin Percival In-reply-to: <48339A2E.1060805@freebsd.org> To: Tim Kientzle Message-id: <4835DD9E.9060906@freebsd.org> MIME-version: 1.0 Content-type: text/plain; charset=ISO-8859-1 Content-transfer-encoding: 7bit X-Enigmail-Version: 0.95.5 References: <200805180624.m4I6Ol12077124@repoman.freebsd.org> <48339A2E.1060805@freebsd.org> User-Agent: Thunderbird 2.0.0.9 (X11/20071117) 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 X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 May 2008 20:55:32 -0000 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