This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] Addition of new option '--rlib' in linker for Renesas SH targets
- From: Nick Clifton <nickc at redhat dot com>
- To: GinaV at KPITCummins dot com
- Cc: binutils at sourceware dot org
- Date: Wed, 12 Jul 2006 19:08:33 +0100
- Subject: Re: [PATCH] Addition of new option '--rlib' in linker for Renesas SH targets
Hi Gina,
> Please refer to the link below for the patch I had submitted:
> http://sources.redhat.com/ml/binutils/2006-07/msg00068.html
Sorry for the delay in reviewing this patch.
Unfortunately I do not think that it is ready for inclusion into the
sources yet. The problems that I have with it are:
* The changes to Makefile.am mean that recreating Makefile.in
results a warning message about the convrenesaslib_SOURCES being
defined but not used. You need to add it to the EXTRA_PROGRAMS
definition.
* The copyright comment at the start of convrenesaslib.c has the
wrong address for the FSF and it says that the tool is part of GLD
whereas in fact it is part of the GNU Binutils.
* REPORT_BUGS_TO is defined in the bin-bugs.h header.
* Having the "magic_num" field of the rlib_header structure be of
the "unsigned char" type results in compile time warning messages
when it is used as an argument to strcpy().
* None of the textual output of the program has been
internationalized.
* The --remove-underscore switch is a bad idea. It would be much
cleaner and safer if you just invoke "objcopy
--remove-leading-char" for yourself as a separate step after
completing the conversion to the GNU ar format.
* The indentation and formatting do not quite match up to the GNU
Coding Standard.
* The use of the program_name array looks very strange to me. I
think that it would be much cleaner if you left program_name as
being the value of argv[0] and you used a different variable when
you are constructing the ar command line.
* There are various "magic" constants in some of the malloc() calls
(eg + 50, or + 100) without any explanation of where they come
from. There are also several fixed length arrays that should not
be fixed (eg filepath, module_name).
* The fatal() function should be used in place of the
printf();exit(1); combination currently used.
* [optional] Adding a --verbose switch would be useful. It could
tell the user the command line it is using to invoke "ar", and
also mention when files are being created and deleted.
* [optional] A test case to check that the tool works would be very
helpful.
Cheers
Nick