Re: [RFA] varobj: call CHECK_TYPEDEF

[Daniel Jacobowitz <drow at mvista dot com>]

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  <drow@mvista.com>

	* 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;