[PATCH] Convert 'name' of 'struct varobj' to 'const char *'.

[PATCH] Convert 'name' of 'struct varobj' to 'const char *'.

[Yao Qi]

This patch is to change the type of field 'name' of 'struct varobj'
from 'char *' to 'const char *'.

gdb:

2013-08-29  Yao Qi  <yao@codesourcery.com>

	* mi/mi-cmd-var.c (mi_cmd_var_create): Change local variable
	'expr''s type to 'const char *'.  Cast expr to 'void *'.
	* varobj.c (struct varobj) <name>: Change its type to
	'const char *'.
	(varobj_create): Change type of parameter 'expression' to
	'const char *'.
	(free_variable): Cast var->name to 'void *'.
	* varobj.h (varobj_create): Update declaration.
---
 gdb/mi/mi-cmd-var.c |    4 ++--
 gdb/varobj.c        |    8 ++++----
 gdb/varobj.h        |    2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index 57a2f6b..f00908d 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -101,7 +101,7 @@ mi_cmd_var_create (char *command, char **argv, int argc)
   struct varobj *var;
   char *name;
   char *frame;
-  char *expr;
+  const char *expr;
   struct cleanup *old_cleanups;
   enum varobj_type var_type;
 
@@ -117,7 +117,7 @@ mi_cmd_var_create (char *command, char **argv, int argc)
   make_cleanup (xfree, frame);
 
   expr = xstrdup (argv[2]);
-  make_cleanup (xfree, expr);
+  make_cleanup (xfree, (void *) expr);
 
   if (strcmp (name, "-") == 0)
     {
diff --git a/gdb/varobj.c b/gdb/varobj.c
index d4fa6ba..55cd83e 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -126,7 +126,7 @@ struct varobj
      child, then this name will be the child's source name.
      (bar, not foo.bar).  */
   /* NOTE: This is the "expression".  */
-  char *name;
+  const char *name;
 
   /* Alloc'd expression for this child.  Can be used to create a
      root variable corresponding to this child.  */
@@ -605,8 +605,8 @@ find_frame_addr_in_frame_chain (CORE_ADDR frame_addr)
 }
 
 struct varobj *
-varobj_create (char *objname,
-	       char *expression, CORE_ADDR frame, enum varobj_type type)
+varobj_create (char *objname, const char *expression, CORE_ADDR frame,
+	       enum varobj_type type)
 {
   struct varobj *var;
   struct cleanup *old_chain;
@@ -2550,7 +2550,7 @@ free_variable (struct varobj *var)
       xfree (var->root);
     }
 
-  xfree (var->name);
+  xfree ((void *) var->name);
   xfree (var->obj_name);
   xfree (var->print_value);
   xfree (var->path_expr);
diff --git a/gdb/varobj.h b/gdb/varobj.h
index 05b2c94..db73748 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -91,7 +91,7 @@ DEF_VEC_O (varobj_update_result);
 /* API functions */
 
 extern struct varobj *varobj_create (char *objname,
-				     char *expression, CORE_ADDR frame,
+				     const char *expression, CORE_ADDR frame,
 				     enum varobj_type type);
 
 extern char *varobj_gen_name (void);
-- 
1.7.7.6

Re: [PATCH] Convert 'name' of 'struct varobj' to 'const char *'.

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> This patch is to change the type of field 'name' of 'struct varobj'
Yao> from 'char *' to 'const char *'.

It's tempting to do this but I think the new casts tell us that
something fishy is going on.

Yao>  struct varobj *
Yao> -varobj_create (char *objname,
Yao> -	       char *expression, CORE_ADDR frame, enum varobj_type type)
Yao> +varobj_create (char *objname, const char *expression, CORE_ADDR frame,
Yao> +	       enum varobj_type type)

Here we're claiming that this takes a const char *.

Yao> -  xfree (var->name);
Yao> +  xfree ((void *) var->name);

But here we cast away const to free the object.

I think this means the object wasn't "really const".  It's an
abstraction violation.

A different approach might be to change varobj_create to accept a const,
and then xstrdup it there.  But then the field still could not really be
const.

Tom

Re: [PATCH] Convert 'name' of 'struct varobj' to 'const char *'.

[Yao Qi]

On 08/31/2013 12:20 AM, Tom Tromey wrote:
> Yao> -  xfree (var->name);
> Yao> +  xfree ((void *) var->name);
>
> But here we cast away const to free the object.
>
> I think this means the object wasn't "really const".  It's an
> abstraction violation.
Tom,
What do you mean by "really const"?  We allocate a string for the name
of var, use type "const char *", which means we can't modify the
contents.  In the end, we xfree (var->name) as a step to release var.
We need a cast because we'll get a warning otherwise,

../../../git/gdb/varobj.c:2531:3: error: passing argument 1 of ‘xfree’ 
discards ‘const’ qualifier from pointer target type [-Werror]
../../../git/gdb/common/common-utils.h:38:6: note: expected ‘void *’ but 
argument is of type ‘const char *’

Am I missing something?

-- 
Yao (齐尧)

Re: [PATCH] Convert 'name' of 'struct varobj' to 'const char *'.

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> On 08/31/2013 12:20 AM, Tom Tromey wrote:
Yao> -  xfree (var->name);
Yao> +  xfree ((void *) var->name);
>> 
>> But here we cast away const to free the object.
>> 
>> I think this means the object wasn't "really const".  It's an
>> abstraction violation.

Yao> What do you mean by "really const"?

What, that isn't defined by the standard?  :-)

What I mean is that the code in the patch has a function that accepts a
"const char *", but the caller must know that this actually means that
only malloc'd strings are acceptable and that there is a transfer of
ownership.

I think this is an unusual use of const.

Tom

Re: [PATCH] Convert 'name' of 'struct varobj' to 'const char *'.

[Yao Qi]

On 09/10/2013 02:02 AM, Tom Tromey wrote:
> What, that isn't defined by the standard?:-)
>
> What I mean is that the code in the patch has a function that accepts a
> "const char *", but the caller must know that this actually means that
> only malloc'd strings are acceptable and that there is a transfer of
> ownership.
>
> I think this is an unusual use of const.

OK, thanks for the explanation.  I withdraw this patch then.

-- 
Yao (齐尧)