Date: Thu, 16 May 2002 15:38:35 +0300 From: Giorgos Keramidas <keramida@ceid.upatras.gr> To: Bruce Evans <bde@zeta.org.au> Cc: Mike Makonnen <makonnen@pacbell.net>, freebsd-audit@FreeBSD.org Subject: Re: RFC: Port of NetBSD cat(1)'s -f option. Message-ID: <20020516123835.GA93447@hades.hell.gr> In-Reply-To: <20020516164332.B1704-100000@gamplex.bde.org> References: <20020515211758.GB68380@hades.hell.gr> <20020516164332.B1704-100000@gamplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2002-05-16 17:40, Bruce Evans <bde@zeta.org.au> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020516123835.GA93447>