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

[Vladimir Prus <ghost at cs dot msu dot 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