This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]