bug#6555: stat enhancement

[A Burgie]

I have received confirmation of the completion of my exciting
paperwork.  I can send the PDF for proof but at this point all of the
legal stuff should be handled.

Thanks,
AB

On Thu, Jul 15, 2010 at 20:08, A Burgie <address@hidden> wrote:
> Attached is the finished DIFF which I believe meets all of the
> requirements.  For notes the problem with compilation had to do with
> the definition of the function in the find-mount-point.h and
> find-mount-point.c files having the 'static' as part of the
> definition.  Removing that allowed compilation to continue.
> Apparently in C something defined as static cannot be used outside
> that C file, even if included.
>
> Let me know if anything else must be done.  I started the licensing
> process but have not heard back at all since I did so.
>
> e0abccac5fbbe1af452cc04f7980f8e7  DIFF0
> 140caa8e1f0e04c8daa42ced5caa0962537ae2a8  DIFF0
>
> Thanks,
> Aaron
>
> On Wed, Jul 7, 2010 at 19:55, A Burgie <address@hidden> wrote:
>> On Wed, Jul 7, 2010 at 00:19, Jim Meyering <address@hidden> wrote:
>>> A Burgie wrote:
>>> 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
>>
>> Sorry... old version of NEWS.  It wasn't really there at the time and
>> will not be in the future.
>>
>>>> -  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.
>>
>> rebasing is my friend.... rebasing is my friend.  It'll be right by
>> shipping time.
>>
>>>> -** 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
>>>> address@hidden %G - Group name of owner
>>>> address@hidden %h - Number of hard links
>>>> address@hidden %i - Inode number
>>>> address@hidden %m - Mount point
>>>> address@hidden %n - File name
>>>> address@hidden %N - Quoted file name with dereference if symbolic link
>>>> address@hidden %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.
>>
>> Done thoughout.
>>
>>>> +#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-"*".
>>
>> Done.
>>
>>>>    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.
>>
>> Wondered about that.  I see in HACKING it talks about this.  Fixed.
>>
>>>> +      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>.
>>
>> Ah ha.  I added a bit more to actually get things where they are now
>> but that is good to know.
>>
>>>> +#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.
>>
>> Sorry... I copied from an example I found, I think in shred.c, though
>> the second example is wrong while the first is correct.  Noted for the
>> future and fixed.
>>
>>>> +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).
>>
>> Finally figured out about the ':set [no]paste' stuff in vi so I can
>> get around this.  Hopefully fixed once and for all.
>>
>>>
>>> The #ifndef...#endif is supposed to span the contents of the file.
>>
>> Fixed, and suddenly the point of compile-time conditionals comes back to me.
>>
>>>> +
>>>> +/* 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.
>>
>> Done.
>>
>>>> +static char * find_mount_point (const char *, const struct stat *);
>>
>> I think I can figure out a changelog.  The one thing left, though, is
>> a bit less cosmetic.  Something about my makefile (which has the two
>> new _SOURCES sections you suggested) is still not letting make compile
>> everything properly.
>>
>> stat.o: In function `print_stat':
>> /home/aburgemeister/code/coreutils/src/stat.c:687: undefined reference
>> to `find_mount_point'
>> collect2: ld returned 1 exit status
>> make[3]: *** [df] Error 1
>> make[3]: Leaving directory `/home/aburgemeister/code/coreutils/src'
>> make[2]: *** [all] Error 2
>> make[2]: Leaving directory `/home/aburgemeister/code/coreutils/src'
>> make[1]: *** [all-recursive] Error 1
>> make[1]: Leaving directory `/home/aburgemeister/code/coreutils'
>> make: *** [all] Error 2
>>
>