From owner-freebsd-audit Thu May 16 9:29:41 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 59CB637B400 for ; Thu, 16 May 2002 09:29:24 -0700 (PDT) Received: from hades.hell.gr (patr530-a094.otenet.gr [212.205.215.94]) by mailsrv.otenet.gr (8.12.3/8.12.3) with ESMTP id g4GGTIHL007565; Thu, 16 May 2002 19:29:20 +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 g4GGTH6m096362; Thu, 16 May 2002 19:29:17 +0300 (EEST) (envelope-from keramida@ceid.upatras.gr) Received: (from charon@localhost) by hades.hell.gr (8.12.3/8.12.3/Submit) id g4GGTCJs096357; Thu, 16 May 2002 19:29:12 +0300 (EEST) (envelope-from keramida@ceid.upatras.gr) Date: Thu, 16 May 2002 19:29:10 +0300 From: Giorgos Keramidas To: Bruce Evans Cc: freebsd-audit@FreeBSD.org Subject: Re: RFC: Port of NetBSD cat(1)'s -f option. Message-ID: <20020516162909.GA96280@hades.hell.gr> References: <20020516123835.GA93447@hades.hell.gr> <20020516235833.S933-100000@gamplex.bde.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20020516235833.S933-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-17 00:35, Bruce Evans wrote: > On Thu, 16 May 2002, Giorgos Keramidas wrote: > > $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 ;) > > Problems with open() are limited by using the O_NONBLOCK flag in the > -f case, but there can still be problems (open() and/or close() can > have side effects), so I think a stat() should be tried first and > checked later. Side-effects meaning things like the creation of new device nodes in the case of cloning devices, or similar? Hmmm. Relying on stat() to check if a file is a regular file, and then using fstat() to verify this would require something like the following though: struct stat st; int fd; if (stat(pathname, &st) == -1) ... if (S_ISREG(st.st_mode) == 0) ... /* Open the file. */ if (fstat(fd, &st) == -1) ... if (S_ISREG(st.st_mode) == 0) ... This doesn't eliminate the possibility of a race condition, but I have to agree it makes things a bit safer. This can be seen at: http://www.FreeBSD.org/~keramida/diff/2002-05-16.cat,ab > > What is it that makes you think the handling of `i' in the added code > > is bogus? :-/ > > The obfuscated `for' loop wasn't too bad when `i' was incremented in one > place, but `i' is now incremented in 3 places and commented on in two > places. I first noticed the style bug of duplicating the comment. One > comment about a simple increment may be justified because the loop is > obfuscated, but not two. I think I'll turn the while in a for loop too, if this change is made. There is a diff that only changes the while to for at: http://www.FreeBSD.org/~keramida/diff/2002-05-16.cat,ac and a diff both the -f flag part and the while->for change at: http://www.FreeBSD.org/~keramida/diff/2002-05-16.cat,ad I'm more in favour of the ,ad diff, since it looks much better than the rest. Not necessarily as a diff, but as resulting code. -- Giorgos Keramidas - http://www.FreeBSD.org keramida@FreeBSD.org - The Power to Serve To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message