Re: du bogus warning with bind mounts

[Jim Meyering]

Ondrej Oprala wrote:
> Hi, I've changed the du util a bit to reflect
> the proposed solution here
> http://marc.info/?l=coreutils-bug&m=134124896022426&w=2 .

Thanks for working on this!

> I'm not sure about the XDEV checking though. Shouldn't the matching
> st_dev numbers be enough to know both files are on the same device?
> If true, then the check is implied in the di_set_lookup function, isn't it?

This is the code for --one-file-system (-x):

          /* The --one-file-system (-x) option cannot exclude anything
             specified on the command-line.  By definition, it can exclude
             a file or directory only when its device number is different
             from that of its just-processed parent directory, and du does
             not process the parent of a command-line argument.  */
          if (fts->fts_options & FTS_XDEV
              && FTS_ROOTLEVEL < ent->fts_level
              && fts->fts_dev != sb->st_dev)
            excluded = true;

It sounds like you have doubts about it?  If so, please be precise.
Giving an example might help.

> Subject: [PATCH] du: Fix an issue with bogus warnings on bind-mounted
>  directories
>
> * NEWS: Mention the fix.
> * src/du.c: Add a function to read+stat mount points
> and properly check cyclic directory entries.
> ---
>  NEWS     |  3 +++
>  gnulib   |  2 +-
>  src/du.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++---
>  3 files changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index ca4568a..8f85c2b 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -4,6 +4,9 @@ GNU coreutils NEWS                                    -*- 
> outline -*-
>
>  ** Bug fixes
>
> +  du no longer emits bogus warnings when traversing bind-mounted
> +  directory cycles.
> +
>    cksum now prints checksums atomically so that concurrent
>    processes will not intersperse their output.
>    [the bug dates back to the initial implementation]
> diff --git a/gnulib b/gnulib
> index 39cedf6..696aa74 160000
> --- a/gnulib
> +++ b/gnulib
> @@ -1 +1 @@
> -Subproject commit 39cedf6f427350ac47118d231c05a7b73b609f89
> +Subproject commit 696aa74b38f8c109c21e4c01194faab53fb4cefc

Please do not include a gnulib submodule update like this.
To reset the git submodule to the reference commit, run this:

    git submodule update

> diff --git a/src/du.c b/src/du.c
> index 7333941..b915320 100644
> --- a/src/du.c
> +++ b/src/du.c
> @@ -27,6 +27,7 @@
>  #include <getopt.h>
>  #include <sys/types.h>
>  #include <assert.h>
> +#include <mntent.h>
>  #include "system.h"
>  #include "argmatch.h"
>  #include "argv-iter.h"
> @@ -60,9 +61,14 @@ extern bool fts_debug;
>  # define FTS_CROSS_CHECK(Fts)
>  #endif
>
> +#define MTAB "/etc/mtab"

Using <mntent.h> and its functions is not portable.
Please use the gnulib function, read_file_system_list,
from mountlist.c as df.c does.

It would be great if you were to include a new test case
to exercise this, too.  I see that tests/rm/one-file-system
creates a bind mount, so you might want to use it as a model.
Note how it is a root-only test and its cleanup_ functions is
careful to perform the unmount.