From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16733 invoked by alias); 25 Nov 2008 17:23:56 -0000 Received: (qmail 16618 invoked by uid 22791); 25 Nov 2008 17:23:55 -0000 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 25 Nov 2008 17:23:20 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id CB9212A9667; Tue, 25 Nov 2008 12:23:09 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id TiTs18vpgW20; Tue, 25 Nov 2008 12:23:09 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 5EC182A95FB; Tue, 25 Nov 2008 12:23:09 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 3EE0EE7ACD; Tue, 25 Nov 2008 09:23:07 -0800 (PST) Date: Wed, 26 Nov 2008 02:03:00 -0000 From: Joel Brobecker To: Jerome Guitton Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [RFA] "lazily" allocate the raw content of lazy values Message-ID: <20081125172307.GD3946@adacore.com> References: <20081124144810.GA90681@adacore.com> <20081125112638.GE61928@adacore.com> <20081125165319.GA12079@adacore.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20081125165319.GA12079@adacore.com> User-Agent: Mutt/1.4.2.2i 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: 2008-11/txt/msg00694.txt.bz2 I'll join Jerome in thanking Tom for taking time to review this patch, and help improving in. Very very much appreciated. Thanks, Tom! > 2008-11-25 Jerome Guitton > > * value.h (allocate_value_lazy): New function declaration. > (value_free): Remove macro, make it a function. > * value.c (value): Move actual content outside of the memory space > of the struct; add a pointer to this actual content. > (allocate_value_lazy, allocate_value_contents): New function. > (allocate_value): Reimplement using these two new functions. > (value_contents_raw, value_contents_all_raw): If no memory > has been allocated yet for the actual content, allocate it. > (value_contents_all): Resync with struct value's changes. > (value_free): New function. > (value_copy, value_primitive_field): Use new function > allocate_value_lazy to allocate lazy values. > (value_change_enclosing_type): Resync with struct value's changes. > As the value is not reallocated, remove the special handling for > the value chain (now obsolete). > * valops.c (value_at_lazy): Use new function allocate_value_lazy. > (value_fetch_lazy): Allocate value content. Use allocate_value_lazy > to allocate lazy values. > (value_slice): Use allocate_value_lazy to allocate lazy values. Pre-approved after the following minor nits are fixed. Also, we discussed on the phone the possibility of having contents == NULL being equivalent to lazy (thus allowing us to get rid of lazy). But it turns out that it makes the value structure a little less convenient in certain cases (getting the value of each element in an array using the same value). At least for now, Jerome elected to keep the two concepts separate, but I think a comment explaining that would be useful. This can be done as a followup patch so that people get a chance to see the new comment and make comments. > + /* Actual contents of the value. Target byte-order. NULL or not valid if ^^ 2 spaces here. > +/* Allocate a value that has the correct length for type TYPE. */ > + > +struct value * > +allocate_value (struct type *type) In this case, I think it's important to say in the function description that this function allocates the value contents. To me "has the correct length for type TYPE" is a little unclear (I realize that this comes from the original implementation). > /* Lazy register values with offsets are not supported. */ > if (VALUE_LVAL (arg1) == lval_register && value_lazy (arg1)) > value_fetch_lazy (arg1); > > if (value_lazy (arg1)) > - set_value_lazy (v, 1); > + v = allocate_value_lazy (value_enclosing_type (arg1)); 1 tab instead of 8 spaces. (I hate tabs) > @@ -2969,13 +2974,15 @@ value_slice (struct value *array, int lo > slice_range_type); > TYPE_CODE (slice_type) = TYPE_CODE (array_type); > > - slice = allocate_value (slice_type); > if (VALUE_LVAL (array) == lval_memory && value_lazy (array)) > - set_value_lazy (slice, 1); > + slice = allocate_value_lazy (slice_type); Same here... -- Joel