From owner-freebsd-audit Thu May 16 5:39:59 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mailsrv.otenet.gr (mailsrv.otenet.gr [195.170.0.5]) by hub.freebsd.org (Postfix) with ESMTP id 0990237B406 for ; Thu, 16 May 2002 05:39:53 -0700 (PDT) Received: from hades.hell.gr (patr530-b220.otenet.gr [212.205.244.228]) by mailsrv.otenet.gr (8.12.3/8.12.3) with ESMTP id g4GCdnHL019424; Thu, 16 May 2002 15:39:50 +0300 (EEST) Received: from hades.hell.gr (hades [127.0.0.1]) by hades.hell.gr (8.12.3/8.12.3) with ESMTP id g4GCdm6m093572; Thu, 16 May 2002 15:39:48 +0300 (EEST) (envelope-from keramida@ceid.upatras.gr) Received: (from charon@localhost) by hades.hell.gr (8.12.3/8.12.3/Submit) id g4GCcaNh093522; Thu, 16 May 2002 15:38:36 +0300 (EEST) (envelope-from keramida@ceid.upatras.gr) Date: Thu, 16 May 2002 15:38:35 +0300 From: Giorgos Keramidas To: Bruce Evans Cc: Mike Makonnen , freebsd-audit@FreeBSD.org Subject: Re: RFC: Port of NetBSD cat(1)'s -f option. Message-ID: <20020516123835.GA93447@hades.hell.gr> References: <20020515211758.GB68380@hades.hell.gr> <20020516164332.B1704-100000@gamplex.bde.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20020516164332.B1704-100000@gamplex.bde.org> User-Agent: Mutt/1.3.99i Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 2002-05-16 17:40, Bruce Evans wrote: > > > + if (stat(path, &st) < 0) { > > ... > > + if (S_ISREG(st.st_mode) == 0) { > > I object. If stat() fails, then we have no idea about the file type, > so we shouldn't classify it as regular and must set rval so that the > error is reflected in cat's exit status. Letting the old code handle > the error seems best. > > Using stat() instead of fstat() gives some races. NetBSD reduces the > races by checking the mode both before and after open(). I have the following version of basesrc/bin/cat locally: $NetBSD: cat.c,v 1.30 2002/05/09 02:13:10 thorpej Exp $ This checks only after the open with fstat(). I agree that using fstat() is better. Thanks for pointing this out ;) > A more fundamental bug: > The new variable fflag is never used. Whoops. That's what one gets for writing C programs very late. Fixed. I had probably meant to add this after the code works with *stat() and forgot all about it, when testing. Thank you again. > Style bugs: > - nested declaration of st. Removed after a comment by Mike Makonnen. > - more verbose and bogus handling of the variable 'i'. 'i' is just the > loop counter for a `for' loop that is obfuscated as a `while' loop. I can't think of some way to use `i' differently, without rewriting the `while' loop as a `for' loop too. Mixing this change with the addition of -f seemed like wrong to me though. The conversion to a `for' loop can be done in a separate change, if it's deemed necessary. What is it that makes you think the handling of `i' in the added code is bogus? :-/ To save the readers of the list a few KB, I won't post the patch again. Its updated version can be found at: http://www.FreeBSD.org/~keramida/diff/2002-05-16.cat,aa - Giorgos To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message