Your message dated Sun, 19 Jan 2020 12:30:56 +0100
with message-id <[email protected]>
and subject line Re:  initscripts: please add code comment regarding 
corner-case mountpoint test (Re: lh_builds breaks without error message)
has caused the Debian Bug report #460898,
regarding initscripts: please add code comment regarding corner-case mountpoint 
test (Re: lh_builds breaks without error message)
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact [email protected]
immediately.)


-- 
460898: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=460898
Debian Bug Tracking System
Contact [email protected] with problems
--- Begin Message ---
Package: initscripts
Version: 2.86.ds1-38
Severity: wishlist

I previously sent to [email protected] the following 

On Thu, Nov 22, 2007 at 08:54:56AM -0500, Justin Pryzby wrote:

> Is it ok to use initscript's /bin/mountpoint instead?  Erm, nvm, the logic in
> that tool seems to be broken (right?):
> 
> |       r = (st.st_dev != st2.st_dev) ||
> |           (st.st_dev == st2.st_dev && st.st_ino == st2.st_ino);
| |             printf("%s is %sa mountpoint\n", path, r ? "" : "not ");
| |        return r ? 0 : 1;

At first I thought that the test should be the usual-looking:

> |       r = (st.st_dev == st2.st_dev && st.st_ino == st2.st_ino);

However this is actually a test that it's the same file which isn't
what's wanted.

I think the first part of the test is right:

> |       r = (st.st_dev != st2.st_dev) ||

This says: "If a subdirectory is on a different device than its
parent, then some other device must be mounted there".  This is the
common reason why "mountpoint" would return true.

I decided that the 2nd part of the test is *also* right, although it
wasn't intuitive why:

> |           (st.st_dev == st2.st_dev && st.st_ino == st2.st_ino);

This says: "If a subdirectory *is the same node* as its own parent,
then (assuming this is a filesystem that uses inodes in a normal-ish
way or otherwise the same things hold true), then the same thing must
be mounted on the upper directory, *and* the corresponding filesystem
has a directory, the name of which is what's being tested for being a
mountpoint, *and* the same thing that's mounted on the upper directory
is also mounted on that directory".  This is a moderate subtlety of a
corner-case reason why mountpoint would return true.

Is this the right interpretation?

The initial code can effectively test (modulo compiler optimization)
the same equality twice; if (st.st_dev != st2.st_dev) is false, then
it's immediately tested again to see instead if the quantities are
equal (which is guaranteed).  This is one thing that caused me to
think that the code was wrong, since it wasn't(/isn't) clear to me
that this was understood at the time the code was introduced.

The test could actually be written

> |       r = (st.st_dev != st2.st_dev) || (st.st_ino == st2.st_ino);

However this looks just as wrong as the initial code (due to
similarity with the "is-the-same-file" test).  It's not clear to me
how to write this test in a self-documenting way, so I think this
deserves a code comment.



--- End Message ---
--- Begin Message ---
Hello,

I'm not really sure why it was considered a sane thing to reassign
this ancient bug report to util-linux without checking if it
makes sense at all still. Oh well, the mountpoint command is
indeed shipped from util-linux sources these days in debian.

The reported "bug" as stated has since been fixed in the sysvinit
implementation. I'd also like to state that the counterpart of
this in the util-linux (both in mountpoint.c and libmount) are
sufficiently commented already.

I'm thus considering this "bug" fully fixed and closing.

Regards,
Andreas Henriksson

--- End Message ---

Reply via email to