Date: Sun, 29 Aug 2010 03:55:24 -0700 From: Garrett Cooper <gcooper@FreeBSD.org> To: Garrett Cooper <gcooper@freebsd.org> 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 Message-ID: <AANLkTi=%2Bse=VDaBrg=X7VVLDqk1-DmFRTCVomH7in8-j@mail.gmail.com> In-Reply-To: <AANLkTi=C7nshKdKWdhTL7qK2dm=o3M83NyyfF6UHFVe9@mail.gmail.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
On Sun, Aug 29, 2010 at 3:54 AM, Garrett Cooper <gcooper@freebsd.org> wrote:
> On Tue, Mar 30, 2010 at 5:40 PM, Garrett Cooper <gcooper@freebsd.org> wrote:
>> On Tue, Mar 30, 2010 at 12:12 PM, Bruce Evans <brde@optusnet.com.au> wrote:
>>> On Wed, 31 Mar 2010, Bruce Evans wrote:
>>>
>>>> On Tue, 30 Mar 2010, Garrett Cooper wrote:
>>>>
>>>>> Hi,
>>>>> I'm not 100% satisfied with this patch now. Looking back it fails
>>>>> the following case:
>>>>>
>>>>> -P Do not follow symbolic links in the file hierarchy, instead
>>>>> con-
>>>>> sider the symbolic link itself in any comparisons. This is the
>>>>> default.
>>>>
>>>> -P should have the same semantics and description in all utilities. The
>>>> description should not have grammar errors like the above (comma splice).
>>>> ...
>>>> I now see that the grammar error is from the original version of mtree(1),
>>>> and is probably one of the things you don't like. mtree also has -L, but
>>>> not -R or -P or -h. It is not clear how any utility that traverses trees
>>>> can work without a full complement of -[HLPR] or how any utility that
>>>> ...
>>>
>>> Looking at the actual patch, I now see that it is about a completely
>>> different problem. You would only need to understand the amount of
>>> brokenness of -P to see if you need to use lstat(). I think -P is so
>>> broken that mtree on symlinks doesn't work at all and not using lstat()
>>> would be safest.
>>
>> Hmmm... so I take it that this is actually the first step in many to
>> fixing this underlying problem? I suppose I should be opening bugs for
>> all of the itemized issues that you see in mtree(8) so someone can
>> submit patches to fix the utility?
>>
>>> The patch has some style bugs.
>>
>> Please expound on this -- I want to improve my style (without having
>> to rewrite the entire program of course) -- so that it conforms more
>> to the projects overall style rules; of course there are some cases
>> where I can't readily do that (like pkg_install -- ugh), but I'll do
>> my best to make sure that the rules are withheld.
>
> Just for the record, here's the latest patch that I submitted to
> Bruce for this PR.
Dog gone it; attached the wrong patch. This is the right patch..
-Garrett
[-- Attachment #2 --]
Index: excludes.c
===================================================================
--- excludes.c (revision 211961)
+++ 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,18 @@
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 `name' points to a real file. */
+ 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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTi=%2Bse=VDaBrg=X7VVLDqk1-DmFRTCVomH7in8-j>