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

Mike Frysinger
2010-12-14T16:14:53Z

[-- 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 --]