Re: [PATCH v2] gdb: bfin: new port

[Mike Frysinger <vapier@gentoo.org>]

[-- Attachment #1: Type: Text/Plain, Size: 1664 bytes --]

On Tuesday, December 14, 2010 01:02:47 Joel Brobecker wrote:
> > 2010-11-16  Jie Zhang  <jie.zhang@analog.com>
> > 
> > 	* Makefile.in (ALLDEPFILES): Add bfin-tdep.c.
> > 	(HFILES_NO_SRCDIR): Add bfin-tdep.h.
> 
> (ALL_TARGET_OBS): Add bfin-tdep.o
> 
> > 	* NEWS: Mention new Blackfin port.
> > 	* bfin-tdep.c, bfin-tdep.h, config/bfin/bfin.mt: New files.
> > 	* configure.tgt (bfin-*-*): Handle bfin targets.
> 
> The syscall file is also missing.

ugh, sorry about that.  when i was squashing changesets in git, i forgot to 
merge the changelogs too.

> There are a couple of nits, highlighted below.  But the patch is
> pre-approved with those nits fixed as suggested. One little issue,
> though: Please make sure that Eli reviews the change in the NEWS
> file before checking in that part of the patch (it can be done
> separately).

Eli approved the changes as found in the original patch, and they are 
unchanged in this version, so i think that should be sufficient.

> > +#ifdef _DEBUG
> > +      fprintf (stderr, "frameless pc case base %x\n", cache->base);
> > +#endif
> 
> You forgot to remove the #ifdef DEBUG block...

indeed.  i went through your original feedback again to make sure i didnt miss 
anything else, and this seems to be the only one.

> > +/* Return the Blackfin ABI associated with GDBARCH.  */
> > +static inline enum bfin_abi
> > +bfin_abi (struct gdbarch *gdbarch)
> > +{
> > +  return gdbarch_tdep (gdbarch)->bfin_abi;
> > +}
> 
> Please define that function in bfin-tdep.c, and put a declaration in
> bfin-tdep.h instead.

ah, i see what you mean now in your previous feedback
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]