Re: [PATCH] Readelf patch to group common symbols.

["Prafulla Thakare" <PrafullaT at KPITCummins dot com>]

Hi Nick,

Thanks for your comments. 

>Thanks for submitting this patch.  Unfortunately I have some issues 
>with it:
* Why is this feature necessary ?  Surely this information can be
gleaned from 
the switches already supported by readelf ?
--> Yes, this information can be gleaned from the switches already
supported by
readelf. However, following are the reasons for adding this new switch
to readelf.

1. This patch collects the common symbols information which is
scattered. 

2. If common symbol is of array type, then the information about number 
of elements is not explicitly available with any existing readelf
switches. 
The newly added option provides this information to user directly. 

3. With existing options, to get the base type and base size of common
symbol, 
user need to manually browse the readelf output file generated using 
--debug-dump. As we know, --debug-dump generates quite a lot stuff in
output, 
so searching for this information manually can be tedious job. The newly
added 
switch is doing this work for the end user.

* Does KPIT have an FSF assignment in place ?  I could only find one:
  KPIT InfoSystems Ltd.	2002-02-26
  Assigns past and future changes. (tc-sh.c)
  anandk@kpit.com (Anandkumar Keshavan)
-->Yes, KPIT has a corporate FSF assignment.

* The new code in the patch does not follow the GNU Coding Standard.
--> Attached patch is corrected as per GNU coding standard.
Following is the Change log for the attached patches:

2004-12-21  Prafulla Thakare	<prafullat@kpitcummins.com>

	* binutils/readelf.c : Added option --common-symb to display the

        common symbol details if built for debug.
	* binutils/testsuite/binutils-all/readelf.exp : Test case for 
        newly added option --common-symb
	* binutils/testsuite/binutils-all/readelf.P : Expected output
file.
	* binutils/doc/binutils.texi : Help about new option
--common-symb 
        added.
	* binutils/NEWS : Information about new option --common-symb
added.

* Since this patch adds a new feature to readelf there should be 
patches to binutils/NEWS to announce it and binutils/doc/binutils.texi 
to document it.
--> The patches for binutils/NEWS (news.patch) to announce it and
binutils/doc/binutils.texi (binutil_texi.patch) to document it are
attached.

* A binutils testsuite entry to check the new functionality provided 
by the patch would be very welcome.
--> Patch (read_exp.patch) for adding the entry in testsuite is
attached. 
File readelf.P is to be added in
"binutils-2.15/binutils/testsuite/binutils-all" 
folder.

Regards,
Prafulla Thakare,
KPIT Cummins InfoSystems Ltd.
Pune, India

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Free download of GNU based tool-chains for Renesas' SH and H8 Series. 
The following site also offers free technical support to its users. 
Visit http://www.kpitgnutools.com for details. 
Latest versions of KPIT GNU tools are released on Oct 1, 2004.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Attachment: readelf.P
Description: readelf.P

Attachment: read_exp.patch
Description: read_exp.patch

Attachment: comm_sym.patch
Description: comm_sym.patch

Attachment: news.patch
Description: news.patch

Attachment: binutil_texi.patch
Description: binutil_texi.patch