From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9498 invoked by alias); 6 Nov 2010 18:52:04 -0000 Received: (qmail 9482 invoked by uid 22791); 6 Nov 2010 18:52:02 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,SPF_HELO_PASS,TW_CP,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.44.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 06 Nov 2010 18:51:57 +0000 Received: from hpaq13.eem.corp.google.com (hpaq13.eem.corp.google.com [172.25.149.13]) by smtp-out.google.com with ESMTP id oA6Ipsp3005003 for ; Sat, 6 Nov 2010 11:51:55 -0700 Received: from ruffy.mtv.corp.google.com (ruffy.mtv.corp.google.com [172.18.118.116]) by hpaq13.eem.corp.google.com with ESMTP id oA6Iprf0006743 for ; Sat, 6 Nov 2010 11:51:53 -0700 Received: by ruffy.mtv.corp.google.com (Postfix, from userid 67641) id EE5612461A3; Sat, 6 Nov 2010 11:51:52 -0700 (PDT) To: gdb-patches@sourceware.org Subject: [patch] value_change_enclosing_type -> set_value_enclosing_type Message-Id: <20101106185152.EE5612461A3@ruffy.mtv.corp.google.com> Date: Sat, 06 Nov 2010 18:52:00 -0000 From: dje@google.com (Doug Evans) X-System-Of-Record: true 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: 2010-11/txt/msg00106.txt.bz2 Hi. The comment for value_change_enclosing_type has this: "The return value is a pointer to the new version of this value structure." which is not true, the input value is modified as a side-effect. Most callers essentially expect this side-effect. Sure they use the result, but they always (except in one case) have just made a new value so there's no need to create a new value object. The exception is value_full_object. It also has a similar comment: "... if that is different from the enclosing type, create a new value ..." This patch does two things: - rename value_change_enclosing_type to set_value_enclosing_type, and make the result void, for clarity and consistency with other set_value_foo functions - have value_full_object always return a new value if it needs to modify the input value I will check it in in a few days if there are no objections. Tested on amd64-linux, no regressions. 2010-11-06 Doug Evans * value.c (set_value_enclosing_type): Renamed from value_change_enclosing_type. All callers updated. * value.h (set_value_enclosing_type): Update. * valops.c (value_full_object): Always return a copy if we need to make changes to the input value. Index: valops.c =================================================================== RCS file: /cvs/src/src/gdb/valops.c,v retrieving revision 1.256 diff -u -p -r1.256 valops.c --- valops.c 4 Nov 2010 20:26:23 -0000 1.256 +++ valops.c 6 Nov 2010 18:38:16 -0000 @@ -335,7 +335,7 @@ value_cast_pointers (struct type *type, /* No superclass found, just change the pointer type. */ arg2 = value_copy (arg2); deprecated_set_value_type (arg2, type); - arg2 = value_change_enclosing_type (arg2, type); + set_value_enclosing_type (arg2, type); set_value_pointed_to_offset (arg2, 0); /* pai: chk_val */ return arg2; } @@ -569,7 +569,7 @@ value_cast (struct type *type, struct va arg2 = value_copy (arg2); deprecated_set_value_type (arg2, type); - arg2 = value_change_enclosing_type (arg2, type); + set_value_enclosing_type (arg2, type); set_value_pointed_to_offset (arg2, 0); /* pai: chk_val */ return arg2; } @@ -1139,11 +1139,9 @@ value_assign (struct value *toval, struc case lval_internalvar: set_internalvar (VALUE_INTERNALVAR (toval), fromval); val = value_copy (fromval); - val = value_change_enclosing_type (val, - value_enclosing_type (fromval)); + set_value_enclosing_type (val, value_enclosing_type (fromval)); set_value_embedded_offset (val, value_embedded_offset (fromval)); - set_value_pointed_to_offset (val, - value_pointed_to_offset (fromval)); + set_value_pointed_to_offset (val, value_pointed_to_offset (fromval)); return val; case lval_internalvar_component: @@ -1334,8 +1332,7 @@ value_assign (struct value *toval, struc memcpy (value_contents_raw (val), value_contents (fromval), TYPE_LENGTH (type)); deprecated_set_value_type (val, type); - val = value_change_enclosing_type (val, - value_enclosing_type (fromval)); + set_value_enclosing_type (val, value_enclosing_type (fromval)); set_value_embedded_offset (val, value_embedded_offset (fromval)); set_value_pointed_to_offset (val, value_pointed_to_offset (fromval)); @@ -1583,7 +1580,8 @@ value_addr (struct value *arg1) /* This may be a pointer to a base subobject; so remember the full derived object's type ... */ - arg2 = value_change_enclosing_type (arg2, lookup_pointer_type (value_enclosing_type (arg1))); + set_value_enclosing_type (arg2, + lookup_pointer_type (value_enclosing_type (arg1))); /* ... and also the relative position of the subobject in the full object. */ set_value_pointed_to_offset (arg2, value_embedded_offset (arg1)); @@ -1644,7 +1642,7 @@ value_ind (struct value *arg1) /* Re-adjust type. */ deprecated_set_value_type (arg2, TYPE_TARGET_TYPE (base_type)); /* Add embedding info. */ - arg2 = value_change_enclosing_type (arg2, enc_type); + set_value_enclosing_type (arg2, enc_type); set_value_embedded_offset (arg2, value_pointed_to_offset (arg1)); /* We may be pointing to an object of some derived type. */ @@ -3413,7 +3411,8 @@ value_full_object (struct value *argp, /* pai: FIXME -- sounds iffy */ if (full) { - argp = value_change_enclosing_type (argp, real_type); + argp = value_copy (argp); + set_value_enclosing_type (argp, real_type); return argp; } Index: value.c =================================================================== RCS file: /cvs/src/src/gdb/value.c,v retrieving revision 1.114 diff -u -p -r1.114 value.c --- value.c 3 Nov 2010 13:49:37 -0000 1.114 +++ value.c 6 Nov 2010 18:38:16 -0000 @@ -1933,21 +1933,20 @@ value_static_field (struct type *type, i return retval; } -/* Change the enclosing type of a value object VAL to NEW_ENCL_TYPE. - You have to be careful here, since the size of the data area for the value - is set by the length of the enclosing type. So if NEW_ENCL_TYPE is bigger - than the old enclosing type, you have to allocate more space for the data. - The return value is a pointer to the new version of this value structure. */ +/* Change the enclosing type of a value object VAL to NEW_ENCL_TYPE. + You have to be careful here, since the size of the data area for the value + is set by the length of the enclosing type. So if NEW_ENCL_TYPE is bigger + than the old enclosing type, you have to allocate more space for the + data. */ -struct value * -value_change_enclosing_type (struct value *val, struct type *new_encl_type) +void +set_value_enclosing_type (struct value *val, struct type *new_encl_type) { if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val))) val->contents = (gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type)); val->enclosing_type = new_encl_type; - return val; } /* Given a value ARG1 (offset by OFFSET bytes) Index: value.h =================================================================== RCS file: /cvs/src/src/gdb/value.h,v retrieving revision 1.163 diff -u -p -r1.163 value.h --- value.h 3 Nov 2010 13:49:37 -0000 1.163 +++ value.h 6 Nov 2010 18:38:16 -0000 @@ -136,8 +136,9 @@ extern void deprecated_set_value_modifia normally. */ extern struct type *value_enclosing_type (struct value *); -extern struct value *value_change_enclosing_type (struct value *val, - struct type *new_type); +extern void set_value_enclosing_type (struct value *val, + struct type *new_type); + extern int value_pointed_to_offset (struct value *value); extern void set_value_pointed_to_offset (struct value *value, int val); extern int value_embedded_offset (struct value *value);