From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17918 invoked by alias); 23 Jan 2008 22:09:09 -0000 Received: (qmail 17910 invoked by uid 22791); 23 Jan 2008 22:09:08 -0000 X-Spam-Check-By: sourceware.org Received: from viper.snap.net.nz (HELO viper.snap.net.nz) (202.37.101.8) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 23 Jan 2008 22:08:38 +0000 Received: from kahikatea.snap.net.nz (138.62.255.123.dynamic.snap.net.nz [123.255.62.138]) by viper.snap.net.nz (Postfix) with ESMTP id 43B3A3DA5AD; Thu, 24 Jan 2008 11:08:35 +1300 (NZDT) Received: by kahikatea.snap.net.nz (Postfix, from userid 1000) id 08D448FC6D; Thu, 24 Jan 2008 11:08:33 +1300 (NZDT) From: Nick Roberts MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <18327.47840.759734.915147@kahikatea.snap.net.nz> Date: Wed, 23 Jan 2008 22:09:00 -0000 To: "Marc Khouzam" Cc: "Daniel Jacobowitz" , Subject: RE: [Patch] -var-evaluate-expression NAME [FORMAT] In-Reply-To: <6D19CA8D71C89C43A057926FE0D4ADAA04290E59@ecamlmw720.eamcs.ericsson.se> References: <20080123144006.GA5885@caradoc.them.org> <6D19CA8D71C89C43A057926FE0D4ADAA04290E59@ecamlmw720.eamcs.ericsson.se> X-Mailer: VM 7.19 under Emacs 23.0.50.35 X-IsSubscribed: yes 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-01/txt/msg00566.txt.bz2 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