Package: coreutils;
Reported by: A Burgie <dajoker <at> gmail.com>
Date: Fri, 2 Jul 2010 20:55:01 UTC
Severity: normal
Done: Pádraig Brady <P <at> draigBrady.com>
Bug is archived. No further changes may be made.
Message #47 received at 6555 <at> debbugs.gnu.org (full text, mbox):
From: Jim Meyering <jim <at> meyering.net> To: A Burgie <dajoker <at> gmail.com> Cc: 6555 <at> debbugs.gnu.org, Pádraig Brady <P <at> draigbrady.com> Subject: Re: bug#6555: stat enhancement Date: Wed, 07 Jul 2010 08:19:04 +0200
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
> +@item %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 <dajoker <at> gmail.com>
> 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.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.