From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5269 invoked by alias); 2 Feb 2008 11:20:56 -0000 Received: (qmail 5251 invoked by uid 22791); 2 Feb 2008 11:20:53 -0000 X-Spam-Check-By: sourceware.org Received: from zigzag.lvk.cs.msu.su (HELO zigzag.lvk.cs.msu.su) (158.250.17.23) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 02 Feb 2008 11:20:25 +0000 Received: from Debian-exim by zigzag.lvk.cs.msu.su with spam-scanned (Exim 4.50) id 1JLGPw-0002ej-Hr for gdb-patches@sources.redhat.com; Sat, 02 Feb 2008 14:20:21 +0300 Received: from localhost ([127.0.0.1] helo=ip6-localhost) by zigzag.lvk.cs.msu.su with esmtp (Exim 4.50) id 1JLGPo-0002eW-4k; Sat, 02 Feb 2008 14:20:08 +0300 From: Vladimir Prus Subject: RE: [Patch] -var-evaluate-expression NAME [FORMAT] To: Marc Khouzam , gdb-patches@sources.redhat.com Date: Sat, 02 Feb 2008 11:20:00 -0000 References: <20080129214347.GC15063@caradoc.them.org> <6D19CA8D71C89C43A057926FE0D4ADAA04290E7D@ecamlmw720.eamcs.ericsson.se> User-Agent: KNode/0.10.5 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8Bit Message-Id: Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-02/txt/msg00044.txt.bz2 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 > #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