From owner-cvs-src@FreeBSD.ORG Wed May 21 03:42:38 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 DBF1E1065671; Wed, 21 May 2008 03:42:38 +0000 (UTC) (envelope-from kientzle@freebsd.org) Received: from kientzle.com (h-66-166-149-50.snvacaid.covad.net [66.166.149.50]) by mx1.freebsd.org (Postfix) with ESMTP id 9ACAE8FC21; Wed, 21 May 2008 03:42:38 +0000 (UTC) (envelope-from kientzle@freebsd.org) Received: from [10.0.0.128] (p54.kientzle.com [66.166.149.54]) by kientzle.com (8.12.9/8.12.9) with ESMTP id m4L3gctv076957; Tue, 20 May 2008 20:42:38 -0700 (PDT) (envelope-from kientzle@freebsd.org) Message-ID: <48339A2E.1060805@freebsd.org> Date: Tue, 20 May 2008 20:42:38 -0700 From: Tim Kientzle User-Agent: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.7.12) Gecko/20060422 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Colin Percival References: <200805180624.m4I6Ol12077124@repoman.freebsd.org> In-Reply-To: <200805180624.m4I6Ol12077124@repoman.freebsd.org> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit 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: Wed, 21 May 2008 03:42:39 -0000 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 > >