Pádraig Brady <[email protected]> writes:
> bool malloced_bp should be initialized to false,
> as otherwise the if (malloced_bp) free(bp); could be UMR.
Nice catch.
> I don't think me_parent_id always identifies the bind source?
> This wouldn't worko for cross file system binds,
> or if the parent exposes a non-root subtree I think?
> E.g. does this work for the /dev/zero to /home/zero case in the original
> report?
>
> Also I'm worried about that the file name concatenation
> would need to consider parent->me_mntroot.
Good points. It seems to work-ish. After this patch:
$ src/stat -c%m /home/zero /tmp
/dev/zero
/tmp/
On Debian 6 with coreutils-8.6:
$ src/stat -c%m /home/zero /tmp
/dev/zero
/tmp
I assume the trailing slash is from something like:
char *resolved_name = file_name_concat ("/tmp", "/", NULL);
> this stuff is hard to think about,
> so no need to rush it in.
Agreed about it being hard to think about. My feeling is that the
conceptual idea of bind mounts, e.g., what Docker documents, makes them
sound much simpler than they are [1]:
When you use a bind mount, a file or directory on the host machine
is mounted from the host into a container.
Even though the behavior diverges from the documentation currently, the
"buggy" behavior doesn't seem too bad. I could see an implementation
choosing to return the non-bind mount point. Certainly it is better than
returning strange results like "/tmp/".
Given that, I'll hold off working on these patches until post release.
> p.s. the gnulib patch commit message references /proc/self/mountlist,
> which should be /proc/self/mountinfo
Oops. There really are too many mount related files. Perhaps that one
will be a thing someday. :)
Collin
[1] https://docs.docker.com/engine/storage/bind-mounts/
