From owner-freebsd-bugs@FreeBSD.ORG Wed Mar 31 06:50:07 2010 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4A4231065670 for ; Wed, 31 Mar 2010 06:50:07 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 1DBB68FC12 for ; Wed, 31 Mar 2010 06:50:07 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id o2V6o6JY060820 for ; Wed, 31 Mar 2010 06:50:07 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id o2V6o6Se060812; Wed, 31 Mar 2010 06:50:06 GMT (envelope-from gnats) Date: Wed, 31 Mar 2010 06:50:06 GMT Message-Id: <201003310650.o2V6o6Se060812@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Garrett Cooper Cc: 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 Reply-To: Garrett Cooper List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 31 Mar 2010 06:50:07 -0000 The following reply was made to PR bin/144411; it has been noted by GNATS. From: Garrett Cooper To: Garrett Cooper Cc: Bruce Evans , bug-followup Subject: Re: bin/144411: [patch] mtree(8) doesn't reject non-regular files for -X Date: Tue, 30 Mar 2010 23:46:20 -0700 On Tue, Mar 30, 2010 at 11:35 PM, Garrett Cooper wrot= e: > On Tue, Mar 30, 2010 at 5:40 PM, Garrett Cooper wro= te: >> On Tue, Mar 30, 2010 at 12:12 PM, Bruce Evans wro= te: >>> On Wed, 31 Mar 2010, Bruce Evans wrote: >>> >>>> On Tue, 30 Mar 2010, Garrett Cooper wrote: >>>> >>>>> Hi, >>>>> =A0 =A0I'm not 100% satisfied with this patch now. Looking back it fa= ils >>>>> the following case: >>>>> >>>>> =A0 =A0 -P =A0 =A0Do not follow symbolic links in the file hierarchy,= instead >>>>> con- >>>>> =A0 =A0 =A0 =A0 =A0 sider the symbolic link itself in any comparisons= . =A0This is the >>>>> =A0 =A0 =A0 =A0 =A0 default. >>>> >>>> -P should have the same semantics and description in all utilities. = =A0The >>>> description should not have grammar errors like the above (comma splic= e). >>>> ... >>>> 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. =A0mtree also has -L= , but >>>> not -R or -P or -h. =A0It 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. =A0You would only need to understand the amount of >>> brokenness of -P to see if you need to use lstat(). =A0I 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. > > s/withheld/held/ > > I guess the problem was the fact that I didn't use 8-space tabs for > first-level tabs? Hard tabs are fine, correct? Ah, the braces for the single line conditional. Yes, I see what you mean now (fixed). Also, I see what you mean about -P being broken; mtree(8) uses chmod instead of lchmod, chown instead of lchown, etc. This definitely needs fixing and I'll assign separate PRs for it. Thanks, -Garrett