This is the mail archive of the mailing list for the GDB 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: Unreviewed patches

Elena Zannoni wrote:
> For the gdb sh-tdep.c part, I have a few comments.
> There is no need for a prototype. I removed all of those a few months back.

Ok, I'll place the new function so that it needs no prototype.
But you have to keep in mind that that makes the call graph more
rigid, unless you like to move code around a lot.

> You should just move the new function to before the gdbarch_init one.
> Instead of adding another enum, you should use
>   struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
>  and then
>   tdep-><...>_REGNUM
> If you need new register numbers for the dsp case, just add
> them to the tdep structure.

I think the stashing of constants into the tdep structure is basically
wrong.  You separate the register names arrays from the literals
that describe their positions, and you replicate the literals
up to four times.  The tdep structure and the sh_gdbarch_init
function are so large that you have lost track of the things that
really belong in tdep, like sh_show_regs, skip_prologue_hard_way,
and do_pseudo_register.  If you look at other gdb ports, you'll
see that they put only variable stuff in tdep, and use enums
for constants.  The sh gdb register naming scheme also doesn't
scale well, the names are again duplicated multiple times.
I have a scheme that names each register just once, and enumerates
it only once - in an enum, of course.  But I thought we should first
sort out the simulator <-> gdb and gcc -> gdb interfaces before we
start on the gdb internals.
2430 Aztec West / Almondsbury / BRISTOL / BS32 4AQ
T:+44 1454 462330

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