Re: [Patch] readelf -c dump archive index like nm -s

[Nick Clifton <nickc at redhat dot com>]

Hi Shen,

> I made a patch which makes readelf -c dump archive index like nm -s.

Thanks very much for submitting this patch.  Do you have a binutils copyright 
assignment on file with the FSF ?  (I looked but did not find one).  Without 
one we unfortunately cannot accept the patch.

One question in particular comes to mind - why do you want this feature in 
readelf ?  Since it can already be done by "nm -s", as you have pointed out, 
what purpose is served by adding this ability to readelf ?

I do have a few comments on the patch as well:

  * When adding a new feature like this you should make sure that
    it is documented (in binutils/doc/binutils.texi in this case)
    and announced (in binutils/NEWS).

  * Comments should be formatted according to the GNU Coding Standard:
http://www.gnu.org/prep/standards/html_node/Comments.html#Comments
    In particular they should start with a capital letter and end
    with a period followed by two spaces.

    In addition the use of white space, especially around function
    calls, ought to be checked.

  * If the file being examined is not an archive then no error or
    warning message is produced.  It would be better to produce an
    informative message along the lines of "file foo is not an archive
    so its index cannot be displayed".

  * You go to a lot of trouble using fseek and ftell to remember the
    current file pointer location inside process_archive.  It would
    be much cleaner if you simply passed a pointer to the header that
    has already been read at the start of process_archive to
    process_archive_index and skipped all of this seeking about.

  * The format of the output is not particularly pretty.  I agree
    that it is the same format as provided by nm, but I am not sure
    that is such a good idea.  Maybe if the output was alpha sorted
    (on object file and then symbol name) and indented to some degree
    then it would be more useful.  eg:

     Index of archive foo.a:
       Binary bar.o contains:
         a_sym
         b_sym
         d_sym
       Binary baz.o contains:
         c_sym
         e_sym

Cheers
  Nick