This is the mail archive of the gdb-patches@sourceware.org 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: [RFC][2/2] Rework value_from_register


"Ulrich Weigand" <uweigand@de.ibm.com> writes:
> I've considered one interface alternative; the patch below has
> common code generate the struct value and fill in default values;
> the gdbarch_value_from_register routine gets the struct value is
> may change things as appropriate.  The alternative would be to
> have gdbarch_value_from_register fully construct the value
> itself (possibly falling back on a common helper routine).
> Any preference on that question?

If we partially initialize the value before passing it to
gdbarch_value_from_register, then all the per-arch functions
implementing that method are potentially dependent on the
initialization; the initialization is effectively part of the
interface of the gdbarch method.  I'd almost prefer passing all the
arguments to value_from_register directly through to the gdbarch
method, and making it allocate the value; value_from_register's
interface has been unchanged as far back as our CVS history goes.

One counter-argument would be if value_from_register currently has
stuff that doesn't belong in a -tdep.c file.  But it seems okay to me.

Could we include a comment for value_from_register in gdbarch.sh?  I
see that the functions around it are not commented, but I (at least)
find it very helpful.  Ideally, the comment should have enough detail
for you to understand how to use the method, without having to look
much at its implementations or uses.

Other than that, I think this looks great.  Thank you for doing all
the revisions you've already done --- I'm sorry for not reviewing more
promptly.  Tested with no regressions on IA-32 Fedora Core 6.  If my
suggestions seem reasonable, please revise and commit.


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