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

Vladimir Prus
2008-02-02T11:20:56Z

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