bug#6555: stat enhancement

[Jim Meyering]

A Burgie wrote:
> Well I am closer, but am still technically nowhere since it still will
> not compile.  The problem with the odd declaration error was because I
> was trying to declare a c-pointer in the case section of the switch
> statement.  I guess that's not allowed in C (back to the easy
> languages for me).  Despite this I am still stuck with the compilation
> complaining about find_mount_point's lack of existence though I'd bet
> a nickel I have everything setup properly.  Attached are a couple DIFF
> files for those who may want to try to replay things and laugh at my
> mediocrity on the condition that an explanation is provided where I'm
> failing.  I'd have just one DIFF file but I can't figure out how to
> make Git show what I want so I think the combination of these two show
> it all (first use of Git as well).
>
> Sorry to be slow at this.  I'd like to learn and have yet to find
> documentation on GNU's site (or anywhere else) other than the diff for
> operand2sig which helped me a little but not quite enough for what
> seems like a very simple operation (extract function, put in file,
> create .h file, include .h file in previous .c files, modify
> Makefile.am, compile).

Start by reading README* and HACKING.
The GNU Coding Standards has plenty of useful background info.
Run "info standards" or see http://www.gnu.org/prep/standards/.

...
>  ** New features
>
> +  stat now shows the mountpoint of a specified file or directory
> +  in its default output.  It also will show this when a format is
> +  explicitly specified through the use of the %m specifier.

As discussed, I'd rather not change the default output.

  stat can now print the mount point of a file via its new %m format directive

> -  du now uses less than half as much memory when operating on trees
> -  with many hard-linked files.  With --count-links (-l), or when
> -  operating on trees with no hard-linked files, there is no change.

Oops.  Your patch would revert the two recent additions to NEWS,
above and below.

> -** Bug fixes
> -
> -  du no longer multiply counts a file that is a directory or whose
> -  link count is 1, even if the file is reached multiple times by
> -  following symlinks or via multiple arguments.
>
>  * Noteworthy changes in release 8.5 (2010-04-23) [stable]
>
> diff --git a/doc/coreutils.texi b/doc/coreutils.texi
> index 21cf36d..ea3f142 100644
> --- a/doc/coreutils.texi
> +++ b/doc/coreutils.texi
> @@ -10666,6 +10666,7 @@ The valid @var{format} directives for files with 
> @option{--format} and
>  @item %G - Group name of owner
>  @item %h - Number of hard links
>  @item %i - Inode number
> address@hidden %m - Mount point
>  @item %n - File name
>  @item %N - Quoted file name with dereference if symbolic link
>  @item %o - I/O block size
> diff --git a/gnulib b/gnulib
> --- a/gnulib
> +++ b/gnulib
> @@ -1 +1 @@
> -Subproject commit 7773f84fe1aa3bb17defad704ee87f2615894ae4
> +Subproject commit 7773f84fe1aa3bb17defad704ee87f2615894ae4-dirty
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 0630a06..f090087 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -145,6 +145,7 @@ noinst_HEADERS =  \
>    copy.h             \
>    cp-hash.h          \
>    dircolors.h                \
> +  findmountpoint.h   \
>    fs.h                       \
>    group-list.h               \
>    ls.h                       \
> diff --git a/src/stat.c b/src/stat.c
> index c3730f0..f283437 100644
> --- a/src/stat.c
> +++ b/src/stat.c
> @@ -68,6 +68,9 @@
>  #include "quotearg.h"
>  #include "stat-time.h"
>  #include "strftime.h"
> +#include "findmountpoint.h"

nameslikethis are hard to read.
I prefer find-mount-point.h.

> +#include "save-cwd.h"
> +#include "xgetcwd.h"
>
>  #if USE_STATVFS
>  # define STRUCT_STATVFS struct statvfs
> @@ -612,6 +615,7 @@ print_stat (char *pformat, size_t prefix_len, char m,
>    struct stat *statbuf = (struct stat *) data;
>    struct passwd *pw_ent;
>    struct group *gw_ent;
> +  char * mp;

Remove the space-after-"*".

>    bool fail = false;
>
>    switch (m)
> @@ -679,6 +683,14 @@ print_stat (char *pformat, size_t prefix_len, char m,
>      case 't':
>        out_uint_x (pformat, prefix_len, major (statbuf->st_rdev));
>        break;
> +    case 'm':
> +      mp = find_mount_point (filename, statbuf);
> +      if (mp) {
> +        out_string (pformat, prefix_len, mp);
> +      } else {
> +        fail = true;
> +      }

Your brace-using style is inconsistent with the rest of the code.
Drop them in this case, since those are one-line "if" and "else" bodies.

> +      break;
>      case 'T':
>        out_uint_x (pformat, prefix_len, minor (statbuf->st_rdev));
>        break;
> @@ -1025,6 +1037,7 @@ The valid format sequences for files (without 
> --file-system):\n\
>        fputs (_("\
>    %h   Number of hard links\n\
>    %i   Inode number\n\
> +  %m   Mount point\n\
>    %n   File name\n\
>    %N   Quoted file name with dereference if symbolic link\n\
>    %o   I/O block size\n\
>
> commit 70d1f1c97f322a164ee872c0399d9fbccc862b18
> Author: Aaron Burgemeister <address@hidden>
> Date:   Tue Jul 6 18:01:53 2010 -0600
>
>     broken-20100707000000Z
>
> diff --git a/src/findmountpoint.c b/src/findmountpoint.c
> new file mode 100644
> index 0000000..665e2fc
> --- /dev/null
> +++ b/src/findmountpoint.c
> @@ -0,0 +1,93 @@

Every .c file must first include <config.h>.

> +#include "save-cwd.h"
> +#include "xgetcwd.h"
> +
> +
> +/* Return the root mountpoint of the file system on which FILE exists, in
> + * malloced storage.  FILE_STAT should be the result of stating FILE.
> + * Give a diagnostic and return NULL if unable to determine the mount point.
> + * Exit if unable to restore current working directory.  */

We don't use this style of comment.
Remove the "*" on continued lines.

> +static char *
> +find_mount_point (const char *file, const struct stat *file_stat)
> +{
> +  struct saved_cwd cwd;
> +  struct stat last_stat;
> +  char *mp = NULL;    /* The malloced mount point.  */
> +
> +  if (save_cwd (&cwd) != 0)
> +  {

You have reindented this function (changing
the brace positioning style to be contrary to the rest of coreutils).

> +    error (0, errno, _("cannot get current directory"));
> +    return NULL;
> +  }
> +
> +  if (S_ISDIR (file_stat->st_mode))
> +    /* FILE is a directory, so just chdir there directly.  */
> +  {
> +    last_stat = *file_stat;
> +    if (chdir (file) < 0)
> +    {
> +      error (0, errno, _("cannot change to directory %s"), quote (file));
> +      return NULL;
> +    }
> +  }
> +  else
> +    /* FILE is some other kind of file; use its directory.  */
> +  {
> +    char *xdir = dir_name (file);
> +    char *dir;
> +    ASSIGN_STRDUPA (dir, xdir);
> +    free (xdir);
> +
> +    if (chdir (dir) < 0)
> +    {
> +      error (0, errno, _("cannot change to directory %s"), quote (dir));
> +      return NULL;
> +    }
> +
> +    if (stat (".", &last_stat) < 0)
> +    {
> +      error (0, errno, _("cannot stat current directory (now %s)"),
> +          quote (dir));
> +      goto done;
> +    }
> +  }
> +
> +  /* Now walk up FILE's parents until we find another file system or /,
> +   * chdiring as we go.  LAST_STAT holds stat information for the last place
> +   * we visited.  */

Same here.

> +  while (true)
> +  {
> +    struct stat st;
> +    if (stat ("..", &st) < 0)
> +    {
> +      error (0, errno, _("cannot stat %s"), quote (".."));
> +      goto done;
> +    }
> +    if (st.st_dev != last_stat.st_dev || st.st_ino == last_stat.st_ino)
> +      /* cwd is the mount point.  */
> +      break;
> +    if (chdir ("..") < 0)
> +    {
> +      error (0, errno, _("cannot change to directory %s"), quote (".."));
> +      goto done;
> +    }
> +    last_stat = st;
> +  }
> +
> +  /* Finally reached a mount point, see what it's called.  */
> +  mp = xgetcwd ();
> +
> +done:
> +  /* Restore the original cwd.  */
> +  {
> +    int save_errno = errno;
> +    if (restore_cwd (&cwd) != 0)
> +      error (EXIT_FAILURE, errno,
> +          _("failed to return to initial working directory"));
> +    free_cwd (&cwd);
> +    errno = save_errno;
> +  }
> +
> +  return mp;
> +}
> +
> +
> diff --git a/src/findmountpoint.h b/src/findmountpoint.h
> new file mode 100644
> index 0000000..3943b1e
> --- /dev/null
> +++ b/src/findmountpoint.h
> @@ -0,0 +1,27 @@
> +/* stat-related time functions.
> + *
> + *    Copyright (C) 2005, 2007, 2009-2010 Free Software Foundation, Inc.
> + *
> + *    This program is free software: you can redistribute it and/or modify
> + *    it under the terms of the GNU General Public License as published by
> + *    the Free Software Foundation; either version 3 of the License, or
> + *    (at your option) any later version.
> + *
> + *    This program is distributed in the hope that it will be useful,
> + *    but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *    GNU General Public License for more details.
> + *
> + *    You should have received a copy of the GNU General Public License
> + *    along with this program.  If not, see <http://www.gnu.org/licenses/>.  
> */

remove the "*"s on continued lines.

> +#ifndef FINDMOUNTPOINT_H
> +#define FINDMOUNTPOINT_H 1
> +#endif

The #ifndef...#endif is supposed to span the contents of the file.

> +
> +/* Return the root mountpoint of the file system on which FILE exists, in
> + * malloced storage.  FILE_STAT should be the result of stating FILE.
> + * Give a diagnostic and return NULL if unable to determine the mount point.
> + * Exit if unable to restore current working directory.  */

Please remove this comment.
It duplicates the one in the .c file.

> +static char * find_mount_point (const char *, const struct stat *);

Once you've addressed all that,
(don't worry about a test.  I'll add that)
you'll want to write a ChangeLog entry.
They're discussed in HACKING.
Run "git log|less" to see many examples.