From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gdb-patches-return-46989-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Received: (qmail 22379 invoked by alias); 4 Jan 2007 20:48:33 -0000
Received: (qmail 22371 invoked by uid 22791); 4 Jan 2007 20:48:33 -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 20:48:28 +0000
Received: (qmail 17981 invoked from network); 4 Jan 2007 20:48:26 -0000
Received: from unknown (HELO localhost) (jimb@127.0.0.2)   by mail.codesourcery.com with ESMTPA; 4 Jan 2007 20:48:26 -0000
To: "Ulrich Weigand" <uweigand@de.ibm.com>
Cc: gdb-patches@sourceware.org,  drow@false.org
Subject: Re: [RFC][1/2] Rework value_from_register
References: <200612081550.kB8FoueU011723@d12av02.megacenter.de.ibm.com>
From: Jim Blandy <jimb@codesourcery.com>
Date: Thu, 04 Jan 2007 20:48:00 -0000
In-Reply-To: <200612081550.kB8FoueU011723@d12av02.megacenter.de.ibm.com> (Ulrich Weigand's message of "Fri, 8 Dec 2006 16:50:56 +0100 (CET)")
Message-ID: <m3vejmtt53.fsf@codesourcery.com>
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: <gdb-patches.sourceware.org>
List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: gdb-patches-owner@sourceware.org
X-SW-Source: 2007-01/txt/msg00138.txt.bz2


"Ulrich Weigand" <uweigand@de.ibm.com> writes:
> ChangeLog:
>
> 	* frame.c (get_frame_register_bytes): New function.
> 	(put_frame_register_bytes): Likewise.
> 	* frame.h (get_frame_register_bytes): Declare.
> 	(put_frame_register_bytes): Likewise.
> 	* findvar.c (value_from_register): Always construct lval_register
> 	values.  Use get_frame_register_bytes.
> 	* valops.c (value_assign): Use get_frame_register_bytes and
> 	put_frame_register_bytes.

This looks great.  Tested without regressions on IA-32 Fedora Core 6.

One suggestion: common practice seems to put detailed explanations of
functions' interfaces in frame.h, and place no comment above the
definition in frame.c.  I think it's better to have one full
explanation, instead of two that can get out of sync.