From: Jim Blandy <jimb@codesourcery.com>
To: "Ulrich Weigand" <uweigand@de.ibm.com>
Cc: gdb-patches@sourceware.org, drow@false.org
Subject: Re: [RFC][2/2] Rework value_from_register
Date: Thu, 04 Jan 2007 21:28:00 -0000 [thread overview]
Message-ID: <m31wmacwhb.fsf@codesourcery.com> (raw)
In-Reply-To: <200612081551.kB8FpiGI012741@d12av02.megacenter.de.ibm.com> (Ulrich Weigand's message of "Fri, 8 Dec 2006 16:51:44 +0100 (CET)")
"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.
next prev parent reply other threads:[~2007-01-04 21:28 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-12-08 15:52 Ulrich Weigand
2006-12-16 1:16 ` Jim Blandy
2006-12-22 17:44 ` Ulrich Weigand
2007-01-04 21:28 ` Jim Blandy [this message]
2007-01-08 20:07 ` Ulrich Weigand
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m31wmacwhb.fsf@codesourcery.com \
--to=jimb@codesourcery.com \
--cc=drow@false.org \
--cc=gdb-patches@sourceware.org \
--cc=uweigand@de.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox