This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [Patch] readelf -c dump archive index like nm -s
- From: Nick Clifton <nickc at redhat dot com>
- To: Shen Feng <shen at cn dot fujitsu dot com>
- Cc: binutils at sourceware dot org
- Date: Fri, 31 Aug 2007 13:45:42 +0100
- Subject: Re: [Patch] readelf -c dump archive index like nm -s
- References: <46D3AF8D.90206@cn.fujitsu.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