bug#6555: stat enhancement

[A Burgie]

On Mon, Jul 5, 2010 at 07:50, Jim Meyering <address@hidden> wrote:
> A Burgie wrote:
>> On Mon, Jul 5, 2010 at 06:48, Jim Meyering <address@hidden> wrote:
>>>> The idea seems sensible.
>>>
>>> I think so, too.
>>> However, the patch that adds the option would do well to add
>>> a test that exercises it and to mention it in the NEWS file.
>>> Your change will qualify as "significant".
>>> Can you file a copyright assignment?  Here are guidelines:
>>>
>>>    http://git.savannah.gnu.org/cgit/coreutils.git/tree/HACKING#n327
>>
>> I was wondering if that would apply, but I did not think it would as
>> it was not really my own code (I copied from previously-GPL'd code and
>> only, perhaps, added three or four lines of my own).  Please confirm
>> if I am mistaken on this.
>
> I figured that between moving that function into a file on its
> own, declaring the function in its own header file,
> adjusting df.c and stat.c to include the new .h file,
> adjusting src/Makefile.am to list the new file, adding a test and
> adding to NEWS you would end up adding more than 10 or 15 lines.

Ah.... I see.  Very well; I'll get to work on the legal side of things.

> Oh.  And documentation.  You'll want to add a line or two to
> the section on stat in doc/coreutils.texi.

Consider it done.

> If it's too much work on the portability front (wouldn't surprise me),
> it may be ok simply to put the statically-declared function body
> in a new .c file and include the .c file from each of stat.c and df.c.

I'll work to do it the right way and let you know if I fail there.

> BTW, please adjust this part of your patch not to dereference NULL
> when find_mount_point fails:
>
>  +    case 'm':
>  +      out_string (pformat, prefix_len, find_mount_point (filename, 
> statbuf));

Any pointers (pun intended) on the best way to do that?  I'll check
the df.c file to see what it does but if it's not there and you have a
better option that'd be appreciated.  I'm not sure it's been pointed
out yet, but I'm not a C expert yet.

> Also, I'm a little reluctant to change the default format to
> add an entire new line just for "Mountpoint: %s\n".
> There is value in not changing the number of lines in the default output:
> compatibility.

Agreed.  My reasoning in doing so was perhaps a bit shortsighted.
Most of the stat output seems to fit nicely within eighty columns and
I did not want to disturb that too much.  Some lines could easily go
outside this (File, Permissions with user/group names, etc.) but File
was on the first line and I was concerned about current users who may
be running stat and then piping output through 'head' or 'tail' to get
just a portion of it.  I could easily enough add Mountpoint after the
filename or anywhere else you feel is better, or it could be removed
from the default output entirely which was what I did originally
before submitting the patch.

Thank-you for your patience,
AB