From owner-freebsd-bugs@FreeBSD.ORG  Wed Oct 13 04:02:17 2010
Return-Path: <owner-freebsd-bugs@FreeBSD.ORG>
Delivered-To: freebsd-bugs@FreeBSD.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id C843E106566C
	for <freebsd-bugs@FreeBSD.org>; Wed, 13 Oct 2010 04:02:17 +0000 (UTC)
	(envelope-from brde@optusnet.com.au)
Received: from mail03.syd.optusnet.com.au (mail03.syd.optusnet.com.au
	[211.29.132.184])
	by mx1.freebsd.org (Postfix) with ESMTP id 5DD8B8FC16
	for <freebsd-bugs@FreeBSD.org>; Wed, 13 Oct 2010 04:02:17 +0000 (UTC)
Received: from c122-106-146-165.carlnfd1.nsw.optusnet.com.au
	(c122-106-146-165.carlnfd1.nsw.optusnet.com.au [122.106.146.165])
	by mail03.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id
	o9D42EKF029593
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO);
	Wed, 13 Oct 2010 15:02:15 +1100
Date: Wed, 13 Oct 2010 15:02:14 +1100 (EST)
From: Bruce Evans <brde@optusnet.com.au>
X-X-Sender: bde@besplex.bde.org
To: Garrett Cooper <gcooper@FreeBSD.org>
In-Reply-To: <AANLkTik6CZKgDsuxoVcyqd9RC3eyFSv2ZV4BsNK7URgP@mail.gmail.com>
Message-ID: <20101013145558.R1817@besplex.bde.org>
References: <201003300830.o2U8U93Y096013@freefall.freebsd.org>
	<20100331034500.O1425@besplex.bde.org>
	<20100331060503.G1425@besplex.bde.org>
	<364299f41003301740m4ca73398v9aadcc87e53a4628@mail.gmail.com>
	<AANLkTi=C7nshKdKWdhTL7qK2dm=o3M83NyyfF6UHFVe9@mail.gmail.com>
	<AANLkTi=+se=VDaBrg=X7VVLDqk1-DmFRTCVomH7in8-j@mail.gmail.com>
	<AANLkTik6CZKgDsuxoVcyqd9RC3eyFSv2ZV4BsNK7URgP@mail.gmail.com>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed
Cc: freebsd-bugs@FreeBSD.org, Bruce Evans <brde@optusnet.com.au>
Subject: Re: bin/144411: [patch] mtree(8) doesn't reject non-regular files
 for -X
X-BeenThere: freebsd-bugs@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: Bug reports <freebsd-bugs.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-bugs>,
	<mailto:freebsd-bugs-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-bugs>
List-Post: <mailto:freebsd-bugs@freebsd.org>
List-Help: <mailto:freebsd-bugs-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-bugs>,
	<mailto:freebsd-bugs-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Wed, 13 Oct 2010 04:02:17 -0000

On Sun, 10 Oct 2010, Garrett Cooper wrote:

> ...
>    I've been sitting on this PR for a while and I'd like to wrap it
> up and move on, if that's ok. Here's a patch with a more suitable
> comment above the stat(2) call.

% Index: usr.sbin/mtree/excludes.c
% ===================================================================
% --- usr.sbin/mtree/excludes.c	(revision 213667)
% +++ usr.sbin/mtree/excludes.c	(working copy)
% @@ -30,9 +30,10 @@
%  #include <sys/cdefs.h>
%  __FBSDID("$FreeBSD$");
% 
% +#include <sys/queue.h>
% +#include <sys/stat.h>
% +#include <sys/time.h>		/* XXX for mtree.h */
%  #include <sys/types.h>
% -#include <sys/time.h>		/* XXX for mtree.h */
% -#include <sys/queue.h>
% 
%  #include <err.h>
%  #include <fnmatch.h>
% @@ -63,11 +64,22 @@
%  void
%  read_excludes_file(const char *name)
%  {
% +	struct stat exclude_stat;
% +	struct exclude *e;
%  	FILE *fp;
%  	char *line, *str;
% -	struct exclude *e;
%  	size_t len;
% 
% +	/* 
% +	 * Make sure that the path we're dealing with points to a regular file,
% +	 * because the exclude list should be a regular file, not a directory,
% +	 * etc.
% +	 */
% +	if (stat(name, &exclude_stat) != 0)
% +		err(EXIT_FAILURE, "stat: %s", name);
% +	if (!S_ISREG(exclude_stat.st_mode))
% +		errx(EXIT_FAILURE, "invalid exclude file: %s", name);
% +
%  	fp = fopen(name, "r");
%  	if (fp == 0)
%  		err(1, "%s", name);

I like the main part of the patch.

The reordering of the includes may be premature or incomplete.  Old
sources include <sys/types.h> first since it was a prerequisite for
all POSIX headers, and most FreeBSD man pages and style(9) still say
to do this although POSIX dropped this requirement in 2001 or earlier
and FreeBSD mostly removed this requirement in ~2002-2003.  So now,
the include of <sys/types.h> might not be needed at all, but it is
hard to tell since there is so much pollution in other headers.  I
would keep the include of <sys/types.h> first if it is kept.

Bruce