Date: Sun, 13 Jan 2013 20:01:06 -0500 From: Lowell Gilbert <freebsd-questions-local@be-well.ilk.org> To: Fbsd8 <fbsd8@a1poweruser.com> Cc: FreeBSD questions <questions@freebsd.org>, Lowell Gilbert <freebsd-questions-local@be-well.ilk.org> Subject: Re: sh script problem with capturing return code Message-ID: <44pq188tnx.fsf@lowell-desk.lan> In-Reply-To: <50EE2604.4020809@a1poweruser.com> (fbsd8@a1poweruser.com's message of "Wed, 09 Jan 2013 21:23:00 -0500") References: <50EC9009.3030305@a1poweruser.com> <20130108224626.8c2d89cd.freebsd@edvax.de> <50EC99F2.3020404@a1poweruser.com> <44d2xevlhb.fsf@lowell-desk.lan> <50ED88CF.7060308@a1poweruser.com> <448v82unxb.fsf@lowell-desk.lan> <444niqum7n.fsf@lowell-desk.lan> <44zk0it6t5.fsf@lowell-desk.lan> <44r4ltu8zp.fsf@lowell-desk.lan> <50EE2604.4020809@a1poweruser.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Fbsd8 <fbsd8@a1poweruser.com> writes:
> Lowell Gilbert wrote:
>> Lowell Gilbert <freebsd-questions-local@be-well.ilk.org> writes:
>>
>>> I think it's a real bug, and the test cases don't cover "extra" elements
>>> at all. Now I just have to figure out the right fix.
>>
>> I'm pretty sure that the fix is just to set rval on jumping to the
>> "extra" tag in vwalk() in src/usr.sbin/mtree/verify.c.
>>
>> But my hot water heater just exploded, so I may not look at code for a
>> few more days.
>
> Hello Lowell,
> Thank you very much.
> I was going crazy trying different combinations of options and script
> logic. I want to thank you for taking my problem seriously and taking
> the time to do your own test cases to verify my findings. Now that I
> know it's a genuine bug in mtree, I can make my plans accordingly. Any
> fix to the mtree utility will take some time to filter down to a
> regular RELEASE. Maybe 9.2 or the big jump to 10.0 by the end of
> 2013. I will leave it up to you the file a PR on this and follow
> through.
> Thanks again, people like you are what makes this questions list so
> valuable and FreeBSD such a great OS. You did outstanding work.
Actually, in retrospect I think it was pretty trivial. I think the
following patch is the right fix for the problem, although I am not
completely certain.
================================================================
Index: /usr/src/usr.sbin/mtree/verify.c
===================================================================
--- /usr/src/usr.sbin/mtree/verify.c (revision 245177)
+++ /usr/src/usr.sbin/mtree/verify.c (working copy)
@@ -149,6 +149,7 @@
}
(void)fts_set(t, p, FTS_SKIP);
}
+ rval = MISMATCHEXIT;
(void)fts_close(t);
if (sflag)
warnx("%s checksum: %lu", fullpath, (unsigned long)crc_total);
================================================================
This fixes the problem that was bothering you, but the interactions of
different features are complicated, and many of them are documented in
fairly loose language.
Would you be interested in extending the test suite for this program?
There are some tests in /usr/src/usr.sbin/mtree/test/, but they don't
cover your issue. I'm particularly concerned with interactions between
mtree(8) options like -u, -U, -q, -d, and specification keywords like
"optional", "nochange", and "ignore". I would feel more comfortable if
someone else wrote up new test cases (because programmers generally
don't -- can't -- test their own blind spots), although I'll certainly
do it if no one else does.
I haven't submitted a PR yet, but I'll do so as soon as I've translated
my test case into a form that can be used in the PR.
[As an upside, I've learned about the fts_ family, which I hadn't really
looked at before.]
Be well.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?44pq188tnx.fsf>