On Mon, Jul 5, 2010 at 07:50, Jim Meyering <[email protected]> wrote: > A Burgie wrote: >> On Mon, Jul 5, 2010 at 06:48, Jim Meyering <[email protected]> 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
