RE: [Patch] -var-evaluate-expression NAME [FORMAT]

["Marc Khouzam" <marc.khouzam@ericsson.com>]

> > I think a solution based on 
> > http://sourceware.org/ml/gdb-patches/2008-01/msg00531.html
> > is probably the cleanest.  Can I have an educated opinion on
> > this decision?
>
> This also uses value_get_print_value in c_value_of_variable.  

Did you mean that this solution "also uses print_value" after Vladimir's 
patch?  What we would do is use the format parameter that would be
passed all the way to c_value_of_variable; if the specified format is 
the same as var->format we call print_value (as done in Vladimir's
patch), but if the format is something different, we call
value_get_print_value (var->value, format).  (See below).

> I can't seem to apply your earlier patch as I get spurious characters
> when I save it (= -> =3D, \n -> =20\n, etc) but I wonder if we've made
> things too complicated.

I've redone the patch (slightly cleaner than before).  It's at the bottom.
 
> What happens if you just use value_get_print_value (var->value, var->format)
> directly in mi_cmd_var_evaluate_expression instead of varobj_get_value (var)?

This won't work, because it bypassed all the logic done between the call to
varobj_get_value() and the one to value_get_print_value(); such things as
returning "{...}" for structs and unions, returning "[numChildren]" for arrays;
it would also bypass the checks for var->value == NULL, and value_lazy(), etc.

Marc

==

Indentation is not correct, tests and doc have been removed.
This is just for discussion since I have to wait for my copyright 
assignment anyway.
I did test and run regression successfully.

Index: gdb/varobj.h
===================================================================
RCS file: /cvs/src/src/gdb/varobj.h,v
retrieving revision 1.15
diff -u -r1.15 varobj.h
--- gdb/varobj.h        30 Jan 2008 07:17:31 -0000      1.15
+++ gdb/varobj.h        5 Feb 2008 14:37:03 -0000
@@ -109,6 +109,9 @@
 
 extern int varobj_get_attributes (struct varobj *var);
 
+extern char *varobj_get_formatted_value (struct varobj *var,
+enum varobj_display_formats format);
+
 extern char *varobj_get_value (struct varobj *var);
 
 extern int varobj_set_value (struct varobj *var, char *expression);
Index: gdb/varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.103
diff -u -r1.103 varobj.c
--- gdb/varobj.c        4 Feb 2008 07:49:04 -0000       1.103
+++ gdb/varobj.c        5 Feb 2008 14:37:03 -0000
@@ -217,7 +217,8 @@
 
 static struct value *value_of_child (struct varobj *parent, int index);
 
-static char *my_value_of_variable (struct varobj *var);
+static char *my_value_of_variable (struct varobj *var,
+    enum varobj_display_formats format);
 
 static char *value_get_print_value (struct value *value,
                                    enum varobj_display_formats format);
@@ -242,7 +243,8 @@
 
 static struct type *c_type_of_child (struct varobj *parent, int index);
 
-static char *c_value_of_variable (struct varobj *var);
+static char *c_value_of_variable (struct varobj *var,
+    enum varobj_display_formats format);
 
 /* C++ implementation */
 
@@ -262,7 +264,8 @@
 
 static struct type *cplus_type_of_child (struct varobj *parent, int index);
 
-static char *cplus_value_of_variable (struct varobj *var);
+static char *cplus_value_of_variable (struct varobj *var,
+    enum varobj_display_formats format);
 
 /* Java implementation */
 
@@ -280,7 +283,8 @@
 
 static struct type *java_type_of_child (struct varobj *parent, int index);
 
-static char *java_value_of_variable (struct varobj *var);
+static char *java_value_of_variable (struct varobj *var,
+    enum varobj_display_formats format);
 
 /* The language specific vector */
 
@@ -313,7 +317,8 @@
   struct type *(*type_of_child) (struct varobj * parent, int index);
 
   /* The current value of VAR. */
-  char *(*value_of_variable) (struct varobj * var);
+  char *(*value_of_variable) (struct varobj * var,
+      enum varobj_display_formats format);
 };
 
 /* Array of known source language routines. */
@@ -833,9 +838,16 @@
 }
 
 char *
+varobj_get_formatted_value (struct varobj *var,
+                           enum varobj_display_formats format)
+{
+  return my_value_of_variable (var, format);
+}
+
+char *
 varobj_get_value (struct varobj *var)
 {
-  return my_value_of_variable (var);
+  return my_value_of_variable (var, var->format);
 }
 
 /* Set the value of an object variable (if it is editable) to the
@@ -1770,10 +1782,11 @@
 
 /* GDB already has a command called "value_of_variable". Sigh. */
 static char *
-my_value_of_variable (struct varobj *var)
+my_value_of_variable (struct varobj *var,
+    enum varobj_display_formats format)
 {
   if (var->root->is_valid)
-    return (*var->root->lang->value_of_variable) (var);
+    return (*var->root->lang->value_of_variable) (var, format);
   else
     return NULL;
 }
@@ -2207,7 +2220,8 @@
 }
 
 static char *
-c_value_of_variable (struct varobj *var)
+c_value_of_variable (struct varobj *var,
+    enum varobj_display_formats format)
 {
   /* BOGUS: if val_print sees a struct/class, or a reference to one,
      it will print out its children instead of "{...}".  So we need to
@@ -2252,7 +2266,13 @@
 
            gdb_assert (varobj_value_is_changeable_p (var));
            gdb_assert (!value_lazy (var->value));
-           return xstrdup (var->print_value);
+           
+           /* If the specified format is the current one,
+              we can reuse print_value */
+           if (format == var->format)
+             return xstrdup (var->print_value);
+           else
+             return value_get_print_value (var->value, format);
          }
       }
     }
@@ -2578,7 +2598,8 @@
 }
 
 static char *
-cplus_value_of_variable (struct varobj *var)
+cplus_value_of_variable (struct varobj *var,
+    enum varobj_display_formats format)
 {
 
   /* If we have one of our special types, don't print out
@@ -2586,7 +2607,7 @@
   if (CPLUS_FAKE_CHILD (var))
     return xstrdup ("");
 
-  return c_value_of_variable (var);
+  return c_value_of_variable (var, format);
 }
 

 /* Java */
@@ -2661,9 +2682,10 @@
 }
 
 static char *
-java_value_of_variable (struct varobj *var)
+java_value_of_variable (struct varobj *var,
+    enum varobj_display_formats format)
 {
-  return cplus_value_of_variable (var);
+  return cplus_value_of_variable (var, format);
 }
 

 extern void _initialize_varobj (void);
Index: gdb/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.978
diff -u -r1.978 Makefile.in
--- gdb/Makefile.in     30 Jan 2008 07:17:31 -0000      1.978
+++ gdb/Makefile.in     5 Feb 2008 14:37:03 -0000
@@ -3208,7 +3208,7 @@
        $(mi_getopt_h) $(remote_h)
        $(CC) -c $(INTERNAL_CFLAGS) $(srcdir)/mi/mi-cmd-target.c
 mi-cmd-var.o: $(srcdir)/mi/mi-cmd-var.c $(defs_h) $(mi_cmds_h) $(ui_out_h) \
-       $(mi_out_h) $(varobj_h) $(value_h) $(gdb_string_h)
+       $(mi_out_h) $(varobj_h) $(value_h) $(gdb_string_h) $(mi_getopt_h)
        $(CC) -c $(INTERNAL_CFLAGS) $(srcdir)/mi/mi-cmd-var.c
 mi-console.o: $(srcdir)/mi/mi-console.c $(defs_h) $(mi_console_h) \
        $(gdb_string_h)
Index: gdb/mi/mi-cmd-var.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-var.c,v
retrieving revision 1.45
diff -u -r1.45 mi-cmd-var.c
--- gdb/mi/mi-cmd-var.c 30 Jan 2008 07:17:31 -0000      1.45
+++ gdb/mi/mi-cmd-var.c 5 Feb 2008 14:37:09 -0000
@@ -28,6 +28,7 @@
 #include "value.h"
 #include <ctype.h>
 #include "gdb_string.h"
+#include "mi-getopt.h"
 
 const char mi_no_values[] = "--no-values";
 const char mi_simple_values[] = "--simple-values";
@@ -190,11 +191,33 @@
   return MI_CMD_DONE;
 }
 
+/* Parse a string argument into a format value.  */
+
+static enum varobj_display_formats
+mi_parse_format (const char *arg)
+{
+  int len;
+
+  len = strlen (arg);
+
+  if (strncmp (arg, "natural", len) == 0)
+    return FORMAT_NATURAL;
+  else if (strncmp (arg, "binary", len) == 0)
+    return FORMAT_BINARY;
+  else if (strncmp (arg, "decimal", len) == 0)
+    return FORMAT_DECIMAL;
+  else if (strncmp (arg, "hexadecimal", len) == 0)
+    return FORMAT_HEXADECIMAL;
+  else if (strncmp (arg, "octal", len) == 0)
+    return FORMAT_OCTAL;
+  else
+    error (_("Unknown display format: must be: \"natural\", \"binary\", \"decimal\", \"hexadecimal\", or \"octal\""));
+}
+
 enum mi_cmd_result
 mi_cmd_var_set_format (char *command, char **argv, int argc)
 {
   enum varobj_display_formats format;
-  int len;
   struct varobj *var;
   char *formspec;
 
@@ -211,21 +234,8 @@
   if (formspec == NULL)
     error (_("mi_cmd_var_set_format: Must specify the format as: \"natural\", \"binary\", \"decimal\", \"hexadecimal\", or \"octal\""));
 
-  len = strlen (formspec);
-
-  if (strncmp (formspec, "natural", len) == 0)
-    format = FORMAT_NATURAL;
-  else if (strncmp (formspec, "binary", len) == 0)
-    format = FORMAT_BINARY;
-  else if (strncmp (formspec, "decimal", len) == 0)
-    format = FORMAT_DECIMAL;
-  else if (strncmp (formspec, "hexadecimal", len) == 0)
-    format = FORMAT_HEXADECIMAL;
-  else if (strncmp (formspec, "octal", len) == 0)
-    format = FORMAT_OCTAL;
-  else
-    error (_("mi_cmd_var_set_format: Unknown display format: must be: \"natural\", \"binary\", \"decimal\", \"hexadecimal\", or \"octal\""));
-
+  format = mi_parse_format (formspec);
+  
   /* Set the format of VAR to given format */
   varobj_set_display_format (var, format);
 
@@ -487,16 +497,58 @@
 mi_cmd_var_evaluate_expression (char *command, char **argv, int argc)
 {
   struct varobj *var;
+  enum varobj_display_formats format;
+  int formatFound;
+  int optind;
+  char *optarg;
+    
+  enum opt
+    {
+      OP_FORMAT
+    };
+  static struct mi_opt opts[] =
+  {
+    {"f", OP_FORMAT, 1},
+    { 0, 0, 0 }
+  };
+
+  /* Parse arguments */
+  format = FORMAT_NATURAL;
+  formatFound = 0;
+  optind = 0;
+  while (1)
+    {
+      int opt = mi_getopt ("mi_cmd_var_evaluate_expression", argc, argv, opts, &optind, &optarg);
+      if (opt < 0)
+       break;
+      switch ((enum opt) opt)
+      {
+       case OP_FORMAT:
+         if (formatFound)
+           error (_("mi_cmd_var_evaluate_expression: cannot specify format more than once"));
 
-  if (argc != 1)
-    error (_("mi_cmd_var_evaluate_expression: Usage: NAME."));
+         format = mi_parse_format (optarg);
+         formatFound = 1;
+         break;
+      }
+    }
+
+  if (optind >= argc)
+     error (_("mi_cmd_var_evaluate_expression: Usage: [-f FORMAT] NAME"));
 
+  if (optind < argc - 1)
+    error (_("mi_cmd_var_evaluate_expression: Garbage at end of command"));
+ 
   /* Get varobj handle, if a valid var obj name was specified */
-  var = varobj_get_handle (argv[0]);
+  var = varobj_get_handle (argv[optind]);
   if (var == NULL)
     error (_("mi_cmd_var_evaluate_expression: Variable object not found"));
 
-  ui_out_field_string (uiout, "value", varobj_get_value (var));
+  if (formatFound)
+    ui_out_field_string (uiout, "value", varobj_get_formatted_value (var, format));
+  else
+    ui_out_field_string (uiout, "value", varobj_get_value (var));
+
   return MI_CMD_DONE;
 }
 
@@ -558,9 +610,9 @@
       nv = varobj_list (&rootlist);
       cleanup = make_cleanup (xfree, rootlist);
       if (mi_version (uiout) <= 1)
-        make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
+       make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
       else
-        make_cleanup_ui_out_list_begin_end (uiout, "changelist");
+       make_cleanup_ui_out_list_begin_end (uiout, "changelist");
       if (nv <= 0)
        {
          do_cleanups (cleanup);
@@ -582,9 +634,9 @@
        error (_("mi_cmd_var_update: Variable object not found"));
 
       if (mi_version (uiout) <= 1)
-        cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
+       cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
       else
-        cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changelist");
+       cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changelist");
       varobj_update_one (var, print_values, 1 /* explicit */);
       do_cleanups (cleanup);
     }
@@ -613,26 +665,26 @@
   else if (nc < 0)
     {
       if (mi_version (uiout) > 1)
-        cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+       cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
       ui_out_field_string (uiout, "name", varobj_get_objname(var));
 
       switch (nc)
       {
-        case NOT_IN_SCOPE:
-          ui_out_field_string (uiout, "in_scope", "false");
+       case NOT_IN_SCOPE:
+         ui_out_field_string (uiout, "in_scope", "false");
+         break;
+       case INVALID:
+         ui_out_field_string (uiout, "in_scope", "invalid");
          break;
-        case INVALID:
-          ui_out_field_string (uiout, "in_scope", "invalid");
-         break;
-        case TYPE_CHANGED:
+       case TYPE_CHANGED:
          ui_out_field_string (uiout, "in_scope", "true");
-          ui_out_field_string (uiout, "new_type", varobj_get_type(var));
-          ui_out_field_int (uiout, "new_num_children", 
+         ui_out_field_string (uiout, "new_type", varobj_get_type(var));
+         ui_out_field_int (uiout, "new_num_children", 
                            varobj_get_num_children(var));
          break;
       }
       if (mi_version (uiout) > 1)
-        do_cleanups (cleanup);
+       do_cleanups (cleanup);
     }
   else
     {