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

[Nick Roberts <nickrob at snap dot net dot nz>]

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