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

[Vladimir Prus <ghost@cs.msu.su>]

Marc Khouzam wrote:



> +char *
> +varobj_get_formatted_value (struct varobj *var,
> +                           enum varobj_display_formats format)
> +{
> +  enum varobj_display_formats oldformat;
> +  struct cleanup *old_chain;
> +  char* value;
> +
> +  oldformat = var->format;
> +  old_chain = make_cleanup_set_format (var, oldformat);

I honestly don't like this 'set global format, then cleanup'
trick, but that's something we can fix later.

> +
> +  var->format = format;
> +  value = my_value_of_variable (var);
> +
> +  do_cleanups (old_chain);
> +  return value;
> +}
> +
> /* Set the value of an object variable (if it is editable) to the
> value of the given expression */
> /* Note: Invokes functions that can call error() */
> @@ -1034,7 +1055,7 @@
> value.  */
> changed = 1;
> }
> -          else  if (var->value == NULL && value == NULL)
> +         else  if (var->value == NULL && value == NULL)

Did you do something about indentation here?

> /* Equal. */
> ;
> else if (var->value == NULL || value == NULL)
> @@ -1144,7 +1165,7 @@
> if (type_changed)
> VEC_safe_push (varobj_p, result, *varp);
> 
> -        if (install_new_value ((*varp), new, type_changed))
> +       if (install_new_value ((*varp), new, type_changed))

Likewise.


> @@ -2693,26 +2744,26 @@
> while (*varp != NULL)
> {
> /* global var must be re-evaluated.  */
> -        if ((*varp)->root->valid_block == NULL)
> -        {
> -          struct varobj *tmp_var;
> -
> -          /* Try to create a varobj with same expression.  If we succeed replace
> -             the old varobj, otherwise invalidate it.  */
> -          tmp_var = varobj_create (NULL, (*varp)->name, (CORE_ADDR) 0, USE_CURRENT_FRAME);
> -          if (tmp_var != NULL)
> -            {
> -             tmp_var->obj_name = xstrdup ((*varp)->obj_name);
> -              varobj_delete (*varp, NULL, 0);
> -              install_variable (tmp_var);
> -            }
> -          else
> -              (*varp)->root->is_valid = 0;
> -        }
> -        else /* locals must be invalidated.  */
> -          (*varp)->root->is_valid = 0;
> +       if ((*varp)->root->valid_block == NULL)

Some spurious reformatting again.

> +         {
> +           struct varobj *tmp_var;
> +
> +            /* Try to create a varobj with same expression.  If we succeed replace
> +               the old varobj, otherwise invalidate it.  */
> +           tmp_var = varobj_create (NULL, (*varp)->name, (CORE_ADDR) 0, USE_CURRENT_FRAME);
> +           if (tmp_var != NULL)
> +             {
> +               tmp_var->obj_name = xstrdup ((*varp)->obj_name);
> +               varobj_delete (*varp, NULL, 0);
> +               install_variable (tmp_var);
> +             }
> +           else
> +             (*varp)->root->is_valid = 0;
> +         }
> +       else /* locals must be invalidated.  */
> +         (*varp)->root->is_valid = 0;
> 
> -        varp++;
> +       varp++;
> }
> xfree (all_rootvarobj);
> }
> 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     1 Feb 2008 19:11:55 -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/doc/gdb.texinfo
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
> retrieving revision 1.466
> diff -u -r1.466 gdb.texinfo
> --- gdb/doc/gdb.texinfo 31 Jan 2008 13:38:49 -0000      1.466
> +++ gdb/doc/gdb.texinfo 1 Feb 2008 19:12:00 -0000
> @@ -20095,12 +20095,16 @@
> @subsubheading Synopsis
> 
> @smallexample
> - -var-evaluate-expression @var{name}
> + -var-evaluate-expression [-f @var{format-spec}] @var{name}
> @end smallexample
> 
> Evaluates the expression that is represented by the specified variable
> -object and returns its value as a string.  The format of the
> -string can be changed using the @code{-var-set-format} command.
> +object and returns its value as a string.  The format of the string
> +can be specified with the @samp{-f} option.  The possible values of
> +this option are the same as for @code{-var-set-format}
> +(@pxref{-var-set-format}).  If the @samp{-f} option is not specified,
> +the current display format will be used.  The current display format
> +can be changed using the @code{-var-set-format} command.
> 
> @smallexample
> value=@var{value}
> @@ -20153,7 +20157,7 @@
> object names, all existing variable objects are updated, except
> for frozen ones (@pxref{-var-set-frozen}).  The option
> @var{print-values} determines whether both names and values, or just
> -names are printed.  The possible values of this options are the same
> +names are printed.  The possible values of this option are the same
> as for @code{-var-list-children} (@pxref{-var-list-children}).  It is
> recommended to use the @samp{--all-values} option, to reduce the
> number of MI commands needed on each program stop.
> 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 1 Feb 2008 19:12:01 -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);

Lots of reformatting here as well. The patch looks otherwise
good to me.

- Volodya