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

Nick Roberts
2008-01-23T22:09:09Z

Marc Khouzam writes:
>
> Here is a new version of the patch using Nick's suggestion.
> I had a bit of trouble with the cleanup and am not sure
> this is how it should be done.

Looks good to me.

> Comments?

I'm not the maintainer but I would think it needs:

A couple of tests in mi-var-display.exp (duplicated in mi2-var-display.exp)
Most of us aren't that proficient in DejaGNU/tcl but hack new tests from
existing ones. There are some notes about running the testsuite in gdb/README.

Updating the documentation (gdb.texinfo). Again you can probably infer the
syntax from existing documentation.

>...
> @@ -211,21 +233,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);
^^^
Space. (I see the refactoring now!)

> +
> /* Set the format of VAR to given format */
> varobj_set_display_format (var, format);
>
> @@ -492,16 +501,25 @@
> mi_cmd_var_evaluate_expression (char *command, char **argv, int argc)
> {
> struct varobj *var;
> + enum varobj_display_formats format;
>
> - if (argc != 1)
> - error (_("mi_cmd_var_evaluate_expression: Usage: NAME."));
> + if (argc != 1 &&
> + (argc != 3 || strcmp(argv[1], "-f") != 0))
^^^^

> + error (_("mi_cmd_var_evaluate_expression: Usage: NAME [-f FORMAT]"));

I think this should really use mi_getopt.

> /* Get varobj handle, if a valid var obj name was specified */
> var = varobj_get_handle (argv[0]);
> if (var == NULL)
> error (_("mi_cmd_var_evaluate_expression: Variable object not found"));
>
> - ui_out_field_string (uiout, "value", varobj_get_value (var));
> + if (argc == 3)
> + {
> + format = mi_parse_format(argv[2]);
^^^
Space.

> + 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;
> }
>

>...
> +struct cleanup_set_format_struct
> +{
> + struct varobj *var;
> + enum varobj_display_formats format;
> +};
> +
> +static void
> +do_set_format_cleanup (void *cleanup_set_format)
> +{
> + struct cleanup_set_format_struct *cleanup_struct = cleanup_set_format;
> +
> + varobj_set_display_format(cleanup_struct->var,
^^^
Space.

> + cleanup_struct->format);
> +
> + xfree(cleanup_struct);
^^^
Space.

> +}
> +
> +static struct cleanup *
> +make_cleanup_set_format (struct varobj *var,
> + enum varobj_display_formats format)
> +{
> + struct cleanup_set_format_struct *cleanup_struct;
> +
> + cleanup_struct = XMALLOC (struct cleanup_set_format_struct);
> + cleanup_struct->var = var;
> + cleanup_struct->format = format;
> +
> + return make_cleanup (do_set_format_cleanup, cleanup_struct);
> +}
> +
> /* This returns the type of the variable. It also skips past typedefs
> to return the real type of the variable.

--
Nick http://www.inet.net.nz/~nickrob