Re: [PATCH] more m68k multi-arch

[Kevin Buettner <kevinb@redhat.com>]

On Jun 28, 11:03am, Kevin Buettner wrote:

> On Jun 24,  1:10pm, Grace Sainsbury wrote:
> 
> > +#if !GDB_MULTI_ARCH
> >  #define STORE_STRUCT_RETURN(ADDR, SP) \
> >    { write_register (A1_REGNUM, (ADDR)); }
> >  
> > @@ -221,11 +228,13 @@
> >  	         (TYPE_LENGTH(TYPE) >= 4 ? 0 : 4 - TYPE_LENGTH(TYPE)),	\
> >  	  TYPE_LENGTH(TYPE))
> >  #endif
> > +#endif //multi-arch
> 
> I've noticed the addition of a number of ``#if !GDB_MULTI_ARCH''
> statements in your patch.  Presumably, these are temporary until
> you finish the job, right?  (I'm wondering if it's necessary to
> keep them around at all.)

I've been looking at this some more...

I've noticed the following define near the top of tm-m68k.h:

    #define GDB_MULTI_ARCH 0

Clearly, with this definition in place, you do need to keep the old
definitions around, because those are the ones that will be used!

It seems to me that your testing will be more productive if you do
one of the following:

    1) define GDB_MULTI_ARCH to be GDB_MULTI_ARCH_PARTIAL (1) instead of 0.

or

    2) change the macro definitions being replaced to instead refer to
       the newly defined functions in m68k-tdep.c.

As it stands now, you are only testing that the functions that you've
added to the tdep.c file build (i.e. compile), but GDB won't actually
be using them.  This means that if you make a typo, it won't be
noticed until some later point when GDB_MULTI_ARCH is changed from 0
to some other value.

Kevin