Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 16 May 2002 19:29:10 +0300
From:      Giorgos Keramidas <keramida@ceid.upatras.gr>
To:        Bruce Evans <bde@zeta.org.au>
Cc:        freebsd-audit@FreeBSD.org
Subject:   Re: RFC: Port of NetBSD cat(1)'s -f option.
Message-ID:  <20020516162909.GA96280@hades.hell.gr>
In-Reply-To: <20020516235833.S933-100000@gamplex.bde.org>
References:  <20020516123835.GA93447@hades.hell.gr> <20020516235833.S933-100000@gamplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 2002-05-17 00:35, Bruce Evans <bde@zeta.org.au> 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




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020516162909.GA96280>