This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
RE: RFA: Blackfin Binutils Port Submission
- From: "Catherine Moore" <clm at cm00re dot com>
- To: "'Nick Clifton'" <nickc at redhat dot com>
- Cc: <binutils at sourceware dot org>
- Date: Fri, 30 Sep 2005 11:12:17 -0500
- Subject: RE: RFA: Blackfin Binutils Port Submission
Thanks Nick.
I've made all of the changes that you suggested and have checked in the
port.
I've also submitted the config.sub patch to config-patches as you requested.
Catherine
:-----Original Message-----
:From: Nick Clifton [mailto:nickc@redhat.com]
:Sent: Thursday, September 29, 2005 8:33 AM
:To: Catherine Moore
:Cc: binutils@sourceware.org
:Subject: Re: RFA: Blackfin Binutils Port Submission
:
:Hi Catherine,
:
:> Please let me know if it's okay to commit.
:
:It is. (Very sorry for taking so long to review this patch).
:
:There are a couple of points however:
:
:> * config.sub (bfin): New.
:> * configure.in (bfin-*-*): New.
:
:Changes to the toplevel configure.in file should be approved by the gdb
:and gcc projects as well, and then applied to both the sourceware
:version and the gcc version. Changes to the config.sub file need to be
:submitted to the config project <config-patches@gnu.org> for approval.
:
:> * Makefile.in: Regenerated.
:> * aclocal.m4: Regenerated.
:
:There is no need to include regenerated files in the actual patch
:itself. It just takes up needless room.
:
:---
:
:I also have a few minor comments about pieces of the patch itself:
:
: > bfd/reloc.c
: > + ENUMDOC
: > + ADI Blackfin 16 bit immediate absolute reloc
:
:Please could you add a full stop to the end of comments. It helps to
:make the generated comment slightly more conformant to the GNU Coding
:standard.
:
: > + /* ADI Blackfin 16 bit immediate absolute reloc */
:
:---
:
:Also in a couple of places you have an unnecessary capitalized second
:letter in a comment, eg:
:
: > + ADI Blackfin 16 bit immediate absolute reloc HIgher 16 bits
:
:This should really be:
:
: + ADI Blackfin 16 bit immediate absolute reloc Higher 16 bits
:
:or maybe even:
:
: + ADI Blackfin 16 bit immediate absolute reloc higher 16 bits
:
:unless the word "Higher" has particular significance to the Blackfin port ?
:
:---
:
: > bfd/cpu-bfin.c
: > + ADI Blackfin 16 bit immediate absolute reloc HIgher 16 bits
:
:Please could the comments in this file be converted to conform the GNU
:Coding Standard ? ie with a capitial first letter, a terminating full
:stop and a couple of spaces before the closing */.
:
:In fact as a general point, all of the comments in your patch should be
:checked to make sure that they conform to the standard.
:
:---
:
: > bfd/elf32-bfin.c+ static bfd_reloc_status_type bfin_bfd_reloc
: > + PARAMS ((bfd *, arelent *, asymbol *, PTR, asection *, bfd *,
:char **));
:
:Since we are now only coding for ISO C90 conformance there is no need to
:use the PARAMS macro, and in fact its use is discouraged. In fact most
:of the static function prototypes in this file can be removed providing
:that you reorganise the code so that a static function is defined before
:it is used.
:
:---
:
: > gas/config/bfin-aux.h
: > + /* bfin-aux.h ADI Blackfin Header file for gas^M
:
:It appears that this file contains extraneous carriage returns at the
:end of every line. (Explicitly shown above as ^M).
:
:
:Cheers
: Nick