Re: RFC: Port of NetBSD cat(1)'s -f option.

On 2002-05-16 17:40, Bruce Evans <[email protected]> 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 [email protected]
with "unsubscribe freebsd-audit" in the body of the message