From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7600 invoked by alias); 9 Feb 2013 04:52:45 -0000 Received: (qmail 7579 invoked by uid 22791); 9 Feb 2013 04:52:44 -0000 X-SWARE-Spam-Status: No, hits=-6.2 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 09 Feb 2013 04:52:36 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r194qZa2006788 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 8 Feb 2013 23:52:36 -0500 Received: from psique (ovpn-113-130.phx2.redhat.com [10.3.113.130]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r194qXAx013089; Fri, 8 Feb 2013 23:52:34 -0500 From: Sergio Durigan Junior To: Jan Kratochvil Cc: GDB Patches Subject: Re: [PATCH] Handle bitfields inside inner structs for internalvars References: <20130208204702.GA22467@host2.jankratochvil.net> X-URL: http://www.redhat.com Date: Sat, 09 Feb 2013 04:52:00 -0000 In-Reply-To: <20130208204702.GA22467@host2.jankratochvil.net> (Jan Kratochvil's message of "Fri, 8 Feb 2013 21:47:02 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (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: 2013-02/txt/msg00234.txt.bz2 On Friday, February 08 2013, Jan Kratochvil wrote: > On Wed, 06 Feb 2013 21:39:10 +0100, Sergio Durigan Junior wrote: >> --- a/gdb/testsuite/gdb.base/bitfields.exp >> +++ b/gdb/testsuite/gdb.base/bitfields.exp >> @@ -245,6 +245,31 @@ proc bitfield_at_offset {} { >> gdb_test "print container.two.u3" ".* = 3" >> } >> >> +proc bitfield_internalvar {} { >> + global gdb_prompt >> + >> + # First, we create an internal var holding an instance of >> + # the struct (zeroed out). >> + gdb_test "set \$myvar = \(struct internalvartest\) \{0\}" "" \ > > Backslashes are excessive here, parentheses are not special characters for the > TCL interpretation and this string is not regexp: > gdb_test "set \$myvar = (struct internalvartest) \{0\}" "" \ Thanks for the review. These extra backslashes were added by me while testing a failure that I was seeing, sorry for letting those sneak in. >> --- a/gdb/valops.c >> +++ b/gdb/valops.c >> @@ -1233,11 +1233,18 @@ value_assign (struct value *toval, struct value *fromval) >> VALUE_INTERNALVAR (toval)); >> >> case lval_internalvar_component: >> - set_internalvar_component (VALUE_INTERNALVAR (toval), >> - value_offset (toval), >> - value_bitpos (toval), >> - value_bitsize (toval), >> - fromval); >> + { >> + int offset = value_offset (toval); >> + >> + if (value_bitsize (toval)) >> + offset += value_offset (value_parent (toval)); > > value_address rather tests for value_parent existence; although value_bitsize > is right as value_parent is currently not used elsewhere. > > if (value_parent (toval)) Do you think it's clearer to use `value_parent' here instead of `value_bitsize'? > { > /* VALUE_INTERNALVAR below corresponds to the parent value while > offset is relative to the parent value. */ > gdb_assert (value_parent (value_parent (toval)) == NULL); > offset += value_offset (value_parent (toval)); > } > > And it was not so clear to me on the first look so I have added this comment > if you are OK with it. Thanks, the comment is fine by me. I'll wait for your reply to the question above, and then proceed. -- Sergio