From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16111 invoked by alias); 4 Jan 2007 21:28:26 -0000 Received: (qmail 16103 invoked by uid 22791); 4 Jan 2007 21:28:24 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 04 Jan 2007 21:28:20 +0000 Received: (qmail 20560 invoked from network); 4 Jan 2007 21:28:18 -0000 Received: from unknown (HELO localhost) (jimb@127.0.0.2) by mail.codesourcery.com with ESMTPA; 4 Jan 2007 21:28:18 -0000 To: "Ulrich Weigand" Cc: gdb-patches@sourceware.org, drow@false.org Subject: Re: [RFC][2/2] Rework value_from_register References: <200612081551.kB8FpiGI012741@d12av02.megacenter.de.ibm.com> From: Jim Blandy Date: Thu, 04 Jan 2007 21:28:00 -0000 In-Reply-To: <200612081551.kB8FpiGI012741@d12av02.megacenter.de.ibm.com> (Ulrich Weigand's message of "Fri, 8 Dec 2006 16:51:44 +0100 (CET)") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-01/txt/msg00150.txt.bz2 "Ulrich Weigand" 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.