From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13032 invoked by alias); 19 Jun 2003 19:53:19 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 31257 invoked from network); 19 Jun 2003 19:45:22 -0000 Received: from unknown (HELO crack.them.org) (146.82.138.56) by sources.redhat.com with SMTP; 19 Jun 2003 19:45:22 -0000 Received: from dsl093-172-017.pit1.dsl.speakeasy.net ([66.93.172.17] helo=nevyn.them.org ident=mail) by crack.them.org with asmtp (Exim 3.12 #1 (Debian)) id 19T5MJ-00020f-00; Thu, 19 Jun 2003 14:46:11 -0500 Received: from drow by nevyn.them.org with local (Exim 3.36 #1 (Debian)) id 19T5LO-0002jX-00; Thu, 19 Jun 2003 15:45:14 -0400 Date: Thu, 19 Jun 2003 19:53:00 -0000 From: Daniel Jacobowitz To: Keith Seitz , gdb-patches@sources.redhat.com Subject: Re: [RFA] varobj: call CHECK_TYPEDEF Message-ID: <20030619194513.GA7225@nevyn.them.org> Mail-Followup-To: Keith Seitz , gdb-patches@sources.redhat.com References: <1051215397.1538.43.camel@lindt.uglyboxes.com> <3EA84A9B.5020308@redhat.com> <1051221433.1534.72.camel@lindt.uglyboxes.com> <3EA8629B.50603@redhat.com> <1055362509.1571.63.camel@lindt.uglyboxes.com> <1055378162.1571.98.camel@lindt.uglyboxes.com> <20030612012810.GA21583@nevyn.them.org> <20030619192845.GA2379@nevyn.them.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20030619192845.GA2379@nevyn.them.org> User-Agent: Mutt/1.5.1i X-SW-Source: 2003-06/txt/msg00635.txt.bz2 On Thu, Jun 19, 2003 at 03:28:45PM -0400, Daniel Jacobowitz wrote: > On Wed, Jun 11, 2003 at 09:28:10PM -0400, Daniel Jacobowitz wrote: > > On Wed, Jun 11, 2003 at 05:36:02PM -0700, Keith Seitz wrote: > > > On Wed, 2003-06-11 at 16:49, David Carlton wrote: > > > > I've just gone and looked over the thread and at Keith's patch; I > > > > think the idea is sound, but the implementation isn't. The comments > > > > at the top of get_type say that it's supposed to skip past typedefs, > > > > so calling CHECK_TYPEDEF certainly seems legitimate. But > > > > CHECK_TYPEDEF calls check_typedef, which already goes through chains > > > > of typedefs, so you can get rid of the loop in get_type. > > > > > > Yup, I think you are correct. I'm sure that I was just being laz^Whasty. > > > :-) > > > > David's analysis sounds right to me. I'll look over the actual code > > tomorrow, really I will... > > I don't think David's conversions are quite right; close though. > However, something here is fishy. > > Compare: > /* This returns the type of the variable. This skips past typedefs > and returns the real type of the variable. It also dereferences > pointers and references. > vs. > static struct type * > get_type (struct varobj *var) > { > struct type *type; > type = var->type; > > while (type != NULL && TYPE_CODE (type) == TYPE_CODE_TYPEDEF) > type = TYPE_TARGET_TYPE (type); > > return type; > } > > That doesn't dereference pointers and references! It looks like > get_type got inserted between get_type_deref and its comments? Can you > confirm that it shouldn't dereference? > > Assuming get_type is not supposed to dereference pointers, then this > would work: > > /* This returns the type of the variable. It also skips past typedefs > to return the real type of the variable. > > NOTE: TYPE_TARGET_TYPE should NOT be used anywhere in this file > except within get_target_type and get_type. */ > static struct type * > get_type (struct varobj *var) > { > struct type *type; > type = var->type; > > if (type != NULL) > type = check_typedef (type); > > return type; > } > > /* This returns the target type (or NULL) of TYPE, also skipping > past typedefs, just like get_type (). > > NOTE: TYPE_TARGET_TYPE should NOT be used anywhere in this file > except within get_target_type and get_type. */ > static struct type * > get_target_type (struct type *type) > { > if (type != NULL) > { > type = TYPE_TARGET_TYPE (type); > type = check_typedef (type); > } > > return type; > } > > Test results for that look OK but I don't have insight built at the > moment, so I didn't test it. I take that back. Test results are abominable; everything crashes because I misunderstood the use of get_target_type. Try this patch. -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer 2003-06-19 Daniel Jacobowitz * varobj.c (get_type, get_target_type): Use check_typedef. Index: varobj.c =================================================================== RCS file: /cvs/src/src/gdb/varobj.c,v retrieving revision 1.38 diff -u -p -r1.38 varobj.c --- varobj.c 4 Dec 2002 00:05:54 -0000 1.38 +++ varobj.c 19 Jun 2003 19:42:17 -0000 @@ -1379,9 +1379,8 @@ make_cleanup_free_variable (struct varob return make_cleanup (do_free_variable_cleanup, var); } -/* This returns the type of the variable. This skips past typedefs - and returns the real type of the variable. It also dereferences - pointers and references. +/* This returns the type of the variable. It also skips past typedefs + to return the real type of the variable. NOTE: TYPE_TARGET_TYPE should NOT be used anywhere in this file except within get_target_type and get_type. */ @@ -1391,8 +1390,8 @@ get_type (struct varobj *var) struct type *type; type = var->type; - while (type != NULL && TYPE_CODE (type) == TYPE_CODE_TYPEDEF) - type = TYPE_TARGET_TYPE (type); + if (type != NULL) + type = check_typedef (type); return type; } @@ -1423,8 +1422,8 @@ get_target_type (struct type *type) if (type != NULL) { type = TYPE_TARGET_TYPE (type); - while (type != NULL && TYPE_CODE (type) == TYPE_CODE_TYPEDEF) - type = TYPE_TARGET_TYPE (type); + if (type != NULL) + type = check_typedef (type); } return type;