* [Patch] -var-evaluate-expression NAME [FORMAT]
@ 2008-01-22 20:38 Marc Khouzam
2008-01-23 3:04 ` Nick Roberts
2008-01-23 3:48 ` Daniel Jacobowitz
0 siblings, 2 replies; 33+ messages in thread
From: Marc Khouzam @ 2008-01-22 20:38 UTC (permalink / raw)
To: gdb-patches
Hi,
Here is a patch to add an optional format parameter to -var-evaluate-expression.
-var-evaluate-expression NAME [FORMAT]
This allows a frontend to request a value in a different format without changing
the current format of the variable object.
If the format is not specified, the command behaves as before (current format of
the varObject is used.)
I kind of cheated and used NULL to indicate that the format used should be the
one stored in the varobj structure. Instead, I could have extended the enumeration
varobj_display_formats to have a FORMAT_CURRENT entry, but I didn't like that much.
I did not update tests, run regression or changed the documentation (don't know how
to do any of these things).
Marc
### Eclipse Workspace Patch 1.0
#P gdb
Index: mi/mi-cmd-var.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-var.c,v
retrieving revision 1.43
diff -u -r1.43 mi-cmd-var.c
--- mi/mi-cmd-var.c 1 Jan 2008 22:53:14 -0000 1.43
+++ mi/mi-cmd-var.c 22 Jan 2008 20:22:07 -0000
@@ -57,7 +57,7 @@
ui_out_field_int (uiout, "numchild", varobj_get_num_children (var));
if (mi_print_value_p (varobj_get_gdb_type (var), print_values))
- ui_out_field_string (uiout, "value", varobj_get_value (var));
+ ui_out_field_string (uiout, "value", varobj_get_value (var, NULL));
type = varobj_get_type (var);
if (type != NULL)
@@ -190,11 +190,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 +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);
+
/* Set the format of VAR to given format */
varobj_set_display_format (var, format);
@@ -489,16 +498,24 @@
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 != 2)
+ error (_("mi_cmd_var_evaluate_expression: Usage: NAME [FORMAT]"));
/* 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 == 2)
+ {
+ format = mi_parse_format(argv[1]);
+ ui_out_field_string (uiout, "value", varobj_get_value (var, &format));
+ }
+ else
+ ui_out_field_string (uiout, "value", varobj_get_value (var, NULL));
+
return MI_CMD_DONE;
}
@@ -524,7 +541,7 @@
if (!varobj_set_value (var, expression))
error (_("mi_cmd_var_assign: Could not assign expression to variable object"));
- ui_out_field_string (uiout, "value", varobj_get_value (var));
+ ui_out_field_string (uiout, "value", varobj_get_value (var, NULL));
return MI_CMD_DONE;
}
@@ -645,7 +662,7 @@
cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
ui_out_field_string (uiout, "name", varobj_get_objname (*cc));
if (mi_print_value_p (varobj_get_gdb_type (*cc), print_values))
- ui_out_field_string (uiout, "value", varobj_get_value (*cc));
+ ui_out_field_string (uiout, "value", varobj_get_value (*cc, NULL));
ui_out_field_string (uiout, "in_scope", "true");
ui_out_field_string (uiout, "type_changed", "false");
if (mi_version (uiout) > 1)
Index: varobj.h
===================================================================
RCS file: /cvs/src/src/gdb/varobj.h,v
retrieving revision 1.14
diff -u -r1.14 varobj.h
--- varobj.h 1 Jan 2008 22:53:13 -0000 1.14
+++ varobj.h 22 Jan 2008 20:22:07 -0000
@@ -104,7 +104,8 @@
extern int varobj_get_attributes (struct varobj *var);
-extern char *varobj_get_value (struct varobj *var);
+extern char *varobj_get_value (struct varobj *var,
+ enum varobj_display_formats *format);
extern int varobj_set_value (struct varobj *var, char *expression);
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.99
diff -u -r1.99 varobj.c
--- varobj.c 1 Jan 2008 22:53:13 -0000 1.99
+++ varobj.c 22 Jan 2008 20:22:07 -0000
@@ -221,7 +221,8 @@
static struct value *value_of_child (struct varobj *parent, int index);
-static char *my_value_of_variable (struct varobj *var);
+static char *my_value_of_variable (struct varobj *var,
+ enum varobj_display_formats *format);
static char *value_get_print_value (struct value *value,
enum varobj_display_formats format);
@@ -246,7 +247,8 @@
static struct type *c_type_of_child (struct varobj *parent, int index);
-static char *c_value_of_variable (struct varobj *var);
+static char *c_value_of_variable (struct varobj *var,
+ enum varobj_display_formats *format);
/* C++ implementation */
@@ -266,7 +268,8 @@
static struct type *cplus_type_of_child (struct varobj *parent, int index);
-static char *cplus_value_of_variable (struct varobj *var);
+static char *cplus_value_of_variable (struct varobj *var,
+ enum varobj_display_formats *format);
/* Java implementation */
@@ -284,7 +287,8 @@
static struct type *java_type_of_child (struct varobj *parent, int index);
-static char *java_value_of_variable (struct varobj *var);
+static char *java_value_of_variable (struct varobj *var,
+ enum varobj_display_formats *format);
/* The language specific vector */
@@ -317,7 +321,8 @@
struct type *(*type_of_child) (struct varobj * parent, int index);
/* The current value of VAR. */
- char *(*value_of_variable) (struct varobj * var);
+ char *(*value_of_variable) (struct varobj * var,
+ enum varobj_display_formats *format);
};
/* Array of known source language routines. */
@@ -849,11 +854,10 @@
}
char *
-varobj_get_value (struct varobj *var)
+varobj_get_value (struct varobj *var, enum varobj_display_formats *format)
{
- return my_value_of_variable (var);
+ return my_value_of_variable (var, format);
}
-
/* 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() */
@@ -1786,10 +1790,10 @@
/* GDB already has a command called "value_of_variable". Sigh. */
static char *
-my_value_of_variable (struct varobj *var)
+my_value_of_variable (struct varobj *var, enum varobj_display_formats *format)
{
if (var->root->is_valid)
- return (*var->root->lang->value_of_variable) (var);
+ return (*var->root->lang->value_of_variable) (var, format);
else
return NULL;
}
@@ -2215,8 +2219,10 @@
}
static char *
-c_value_of_variable (struct varobj *var)
+c_value_of_variable (struct varobj *var, enum varobj_display_formats *format)
{
+ enum varobj_display_formats format_to_use;
+
/* BOGUS: if val_print sees a struct/class, or a reference to one,
it will print out its children instead of "{...}". So we need to
catch that case explicitly. */
@@ -2260,7 +2266,12 @@
gdb_assert (varobj_value_is_changeable_p (var));
gdb_assert (!value_lazy (var->value));
- return value_get_print_value (var->value, var->format);
+ if (format == NULL)
+ format_to_use = var->format;
+ else
+ format_to_use = *format;
+
+ return value_get_print_value (var->value, format_to_use);
}
}
}
@@ -2586,7 +2597,7 @@
}
static char *
-cplus_value_of_variable (struct varobj *var)
+cplus_value_of_variable (struct varobj *var, enum varobj_display_formats *format)
{
/* If we have one of our special types, don't print out
@@ -2594,7 +2605,7 @@
if (CPLUS_FAKE_CHILD (var))
return xstrdup ("");
- return c_value_of_variable (var);
+ return c_value_of_variable (var, format);
}
/* Java */
@@ -2669,9 +2680,9 @@
}
static char *
-java_value_of_variable (struct varobj *var)
+java_value_of_variable (struct varobj *var, enum varobj_display_formats *format)
{
- return cplus_value_of_variable (var);
+ return cplus_value_of_variable (var, format);
}
extern void _initialize_varobj (void);
Marc Khouzam
Software Designer, Methods and Tools
Ericsson Canada Inc
EMC/Q
8500 Decarie Blvd.
H4P 2N2, Mont-Royal, Qc, Canada
www.ericsson.comOffice: +514 345 7900 x42350
Fax: +514 345 6159
Mobile: +514 951 7191
Email: Marc.Khouzam@ericsson.com
Ce courriel est confidentiel et uniquement destiné à son ou ses destinataires. Il est défendu de le consulter, de l'utiliser, de le dévoiler ou de le diffuser sans autorisation. Si ce message vous est parvenu par erreur, merci d'en aviser l'expéditeur par retour de courrier et de le détruire sans le divulguer. Un courriel et ses pièces jointes peut être sans autorisation corrompu, interrompu, amendé, altéré et infecté. L'entreprise ne reçoit et n'envoie de courriel qu'avec l'entente qu'elle n'est responsable d'aucune corruption, interception, modification, altération, infection ou conséquence possible.
This communication is confidential and intended solely for the addressee(s). Any unauthorized review, use, disclosure or distribution is prohibited. If you believe this message has been sent to you in error, please notify the sender by replying to this transmission and delete the message without disclosing it. Thank you. E-mail including attachments is susceptible to data corruption, interruption, unauthorized amendment, tampering and viruses, and we only send and receive e-mails on the basis that we are not liable for any such corruption, interception, amendment, tampering or viruses or any consequences thereof.
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-01-22 20:38 [Patch] -var-evaluate-expression NAME [FORMAT] Marc Khouzam @ 2008-01-23 3:04 ` Nick Roberts 2008-01-23 14:30 ` Marc Khouzam 2008-01-23 3:48 ` Daniel Jacobowitz 1 sibling, 1 reply; 33+ messages in thread From: Nick Roberts @ 2008-01-23 3:04 UTC (permalink / raw) To: Marc Khouzam; +Cc: gdb-patches > I kind of cheated and used NULL to indicate that the format used should be > the one stored in the varobj structure. I would use a new function, e.g., something like char * varobj_get_formatted_value (struct varobj *var, enum varobj_display_formats format) { enum varobj_display_formats oldformat; char* value; oldformat = var->format; var->format = format; value = varobj_get_value (var); var->format = oldformat; return value; } and call it when argc == 2 in mi_cmd_var_evaluate_expression so that you don't have to change all calls to varobj_get_value and related functions. (Note: not tested) >... > +/* 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\"")); > +} > + and I would refactor this out of mi_cmd_var_set_format. -- Nick http://www.inet.net.nz/~nickrob ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-01-23 3:04 ` Nick Roberts @ 2008-01-23 14:30 ` Marc Khouzam 2008-01-23 14:42 ` Daniel Jacobowitz 0 siblings, 1 reply; 33+ messages in thread From: Marc Khouzam @ 2008-01-23 14:30 UTC (permalink / raw) To: Nick Roberts; +Cc: gdb-patches > > I kind of cheated and used NULL to indicate that the format used should be > > the one stored in the varobj structure. > > I would use a new function, e.g., something like > > char * > varobj_get_formatted_value (struct varobj *var, > enum varobj_display_formats format) > { > enum varobj_display_formats oldformat; > char* value; > oldformat = var->format; > var->format = format; > value = varobj_get_value (var); > var->format = oldformat; > return value; > } > > and call it when argc == 2 in mi_cmd_var_evaluate_expression so that you > don't have to change all calls to varobj_get_value and related functions. > (Note: not tested) I hadn't thought of temporarily setting var->format, instead of passing 'format' everywhere. Much easier! Is there any way that varob_get_value() could not return? Like a failed gdb_assert? Or are we sure that var->format will be reset to the old format properly no matter what happens to varobj_get_value? > > +/* 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\"")); > > +} > > + > > and I would refactor this out of mi_cmd_var_set_format. Didn't I do that already? ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-01-23 14:30 ` Marc Khouzam @ 2008-01-23 14:42 ` Daniel Jacobowitz 2008-01-23 17:04 ` Marc Khouzam 0 siblings, 1 reply; 33+ messages in thread From: Daniel Jacobowitz @ 2008-01-23 14:42 UTC (permalink / raw) To: Marc Khouzam; +Cc: Nick Roberts, gdb-patches On Wed, Jan 23, 2008 at 09:29:31AM -0500, Marc Khouzam wrote: > I hadn't thought of temporarily setting var->format, instead of passing 'format' > everywhere. Much easier! Is there any way that varob_get_value() could not return? > Like a failed gdb_assert? Or are we sure that var->format will be reset to > the old format properly no matter what happens to varobj_get_value? It may be possible - take a look at GDB's "cleanup" mechanism to handle this safely. -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-01-23 14:42 ` Daniel Jacobowitz @ 2008-01-23 17:04 ` Marc Khouzam 2008-01-23 22:09 ` Nick Roberts 0 siblings, 1 reply; 33+ messages in thread From: Marc Khouzam @ 2008-01-23 17:04 UTC (permalink / raw) To: Daniel Jacobowitz; +Cc: Nick Roberts, gdb-patches 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. Comments? Marc ### Eclipse Workspace Patch 1.0 #P gdb Index: mi/mi-cmd-var.c =================================================================== RCS file: /cvs/src/src/gdb/mi/mi-cmd-var.c,v retrieving revision 1.44 diff -u -r1.44 mi-cmd-var.c --- mi/mi-cmd-var.c 23 Jan 2008 06:13:44 -0000 1.44 +++ mi/mi-cmd-var.c 23 Jan 2008 16:59:00 -0000 @@ -190,11 +190,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 +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); + /* 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]")); /* 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]); + 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; } Index: varobj.h =================================================================== RCS file: /cvs/src/src/gdb/varobj.h,v retrieving revision 1.14 diff -u -r1.14 varobj.h --- varobj.h 1 Jan 2008 22:53:13 -0000 1.14 +++ varobj.h 23 Jan 2008 16:59:00 -0000 @@ -104,6 +104,9 @@ extern int varobj_get_attributes (struct varobj *var); +extern char *varobj_get_formatted_value (struct varobj *var, + enum varobj_display_formats format); + extern char *varobj_get_value (struct varobj *var); extern int varobj_set_value (struct varobj *var, char *expression); Index: varobj.c =================================================================== RCS file: /cvs/src/src/gdb/varobj.c,v retrieving revision 1.99 diff -u -r1.99 varobj.c --- varobj.c 1 Jan 2008 22:53:13 -0000 1.99 +++ varobj.c 23 Jan 2008 16:59:00 -0000 @@ -192,6 +192,9 @@ static struct cleanup *make_cleanup_free_variable (struct varobj *var); +static struct cleanup *make_cleanup_set_format (struct varobj *var, + enum varobj_display_formats format); + static struct type *get_type (struct varobj *var); static struct type *get_value_type (struct varobj *var); @@ -854,6 +857,24 @@ return my_value_of_variable (var); } +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); + + 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() */ @@ -1543,6 +1564,36 @@ return make_cleanup (do_free_variable_cleanup, var); } +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, + cleanup_struct->format); + + xfree(cleanup_struct); +} + +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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-01-23 17:04 ` Marc Khouzam @ 2008-01-23 22:09 ` Nick Roberts 2008-01-24 21:36 ` Marc Khouzam 0 siblings, 1 reply; 33+ messages in thread From: Nick Roberts @ 2008-01-23 22:09 UTC (permalink / raw) To: Marc Khouzam; +Cc: Daniel Jacobowitz, gdb-patches 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-01-23 22:09 ` Nick Roberts @ 2008-01-24 21:36 ` Marc Khouzam 2008-01-25 9:59 ` Eli Zaretskii 2008-01-29 21:45 ` Daniel Jacobowitz 0 siblings, 2 replies; 33+ messages in thread From: Marc Khouzam @ 2008-01-24 21:36 UTC (permalink / raw) To: Nick Roberts; +Cc: Daniel Jacobowitz, gdb-patches New version based on comments. This patch also includes tests and documentation update. I ran regression and got no failure in the new tests that I added. Comments? Marc ### Eclipse Workspace Patch 1.0 #P src Index: gdb/testsuite/gdb.mi/mi-var-display.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-var-display.exp,v retrieving revision 1.21 diff -u -r1.21 mi-var-display.exp --- gdb/testsuite/gdb.mi/mi-var-display.exp 23 Jan 2008 06:20:57 -0000 1.21 +++ gdb/testsuite/gdb.mi/mi-var-display.exp 24 Jan 2008 21:26:46 -0000 @@ -171,6 +171,49 @@ mi_gdb_test "-var-evaluate-expression foo" \ "\\^done,value=\"3\"" \ "eval variable foo" + +# Test: c_variable-6.19 +# Desc: check optional format parameter of var-evaluate-expression +# and check that current format is not changed +mi_gdb_test "-var-evaluate-expression foo -f hex" \ + "\\^done,value=\"0x3\"" \ + "eval variable foo in hex" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in hex" + +mi_gdb_test "-var-evaluate-expression foo -f octal" \ + "\\^done,value=\"03\"" \ + "eval variable foo in octal" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in octal" + +mi_gdb_test "-var-evaluate-expression foo -f decimal" \ + "\\^done,value=\"3\"" \ + "eval variable foo in decimal" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in decimal" + +mi_gdb_test "-var-evaluate-expression foo -f nat" \ + "\\^done,value=\"0x3\"" \ + "eval variable foo in natural" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in natural" + +mi_gdb_test "-var-evaluate-expression foo -f bin" \ + "\\^done,value=\"11\"" \ + "eval variable foo in binary" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in binary" mi_gdb_test "-var-delete foo" \ "\\^done,ndeleted=\"1\"" \ Index: gdb/testsuite/gdb.mi/mi2-var-display.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi2-var-display.exp,v retrieving revision 1.14 diff -u -r1.14 mi2-var-display.exp --- gdb/testsuite/gdb.mi/mi2-var-display.exp 23 Jan 2008 21:05:16 -0000 1.14 +++ gdb/testsuite/gdb.mi/mi2-var-display.exp 24 Jan 2008 21:26:46 -0000 @@ -171,6 +171,49 @@ mi_gdb_test "-var-evaluate-expression foo" \ "\\^done,value=\"3\"" \ "eval variable foo" + +# Test: c_variable-6.19 +# Desc: check optional format parameter of var-evaluate-expression +# and check that current format is not changed +mi_gdb_test "-var-evaluate-expression foo -f hex" \ + "\\^done,value=\"0x3\"" \ + "eval variable foo in hex" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in hex" + +mi_gdb_test "-var-evaluate-expression foo -f octal" \ + "\\^done,value=\"03\"" \ + "eval variable foo in octal" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in octal" + +mi_gdb_test "-var-evaluate-expression foo -f decimal" \ + "\\^done,value=\"3\"" \ + "eval variable foo in decimal" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in decimal" + +mi_gdb_test "-var-evaluate-expression foo -f nat" \ + "\\^done,value=\"0x3\"" \ + "eval variable foo in natural" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in natural" + +mi_gdb_test "-var-evaluate-expression foo -f bin" \ + "\\^done,value=\"11\"" \ + "eval variable foo in binary" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in binary" mi_gdb_test "-var-delete foo" \ "\\^done,ndeleted=\"1\"" \ Index: gdb/varobj.h =================================================================== RCS file: /cvs/src/src/gdb/varobj.h,v retrieving revision 1.14 diff -u -r1.14 varobj.h --- gdb/varobj.h 1 Jan 2008 22:53:13 -0000 1.14 +++ gdb/varobj.h 24 Jan 2008 21:26:41 -0000 @@ -104,6 +104,9 @@ extern int varobj_get_attributes (struct varobj *var); +extern char *varobj_get_formatted_value (struct varobj *var, + enum varobj_display_formats format); + extern char *varobj_get_value (struct varobj *var); extern int varobj_set_value (struct varobj *var, char *expression); Index: gdb/varobj.c =================================================================== RCS file: /cvs/src/src/gdb/varobj.c,v retrieving revision 1.99 diff -u -r1.99 varobj.c --- gdb/varobj.c 1 Jan 2008 22:53:13 -0000 1.99 +++ gdb/varobj.c 24 Jan 2008 21:26:41 -0000 @@ -192,6 +192,9 @@ static struct cleanup *make_cleanup_free_variable (struct varobj *var); +static struct cleanup *make_cleanup_set_format (struct varobj *var, + enum varobj_display_formats format); + static struct type *get_type (struct varobj *var); static struct type *get_value_type (struct varobj *var); @@ -854,6 +857,24 @@ return my_value_of_variable (var); } +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); + + 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() */ @@ -1543,6 +1564,36 @@ return make_cleanup (do_free_variable_cleanup, var); } +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, + cleanup_struct->format); + + xfree (cleanup_struct); +} + +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. Index: gdb/doc/gdb.texinfo =================================================================== RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v retrieving revision 1.459 diff -u -r1.459 gdb.texinfo --- gdb/doc/gdb.texinfo 23 Jan 2008 11:26:29 -0000 1.459 +++ gdb/doc/gdb.texinfo 24 Jan 2008 21:26:46 -0000 @@ -19995,12 +19995,16 @@ @subsubheading Synopsis @smallexample - -var-evaluate-expression @var{name} + -var-evaluate-expression @var{name} [-f @var{format-spec}] @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} @@ -20053,7 +20057,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.44 diff -u -r1.44 mi-cmd-var.c --- gdb/mi/mi-cmd-var.c 23 Jan 2008 06:13:44 -0000 1.44 +++ gdb/mi/mi-cmd-var.c 24 Jan 2008 21:26:46 -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); @@ -492,16 +502,59 @@ 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 } + }; - if (argc != 1) - error (_("mi_cmd_var_evaluate_expression: Usage: NAME.")); - + + if (argc == 0) + error (_("mi_cmd_var_evaluate_expression: Usage: NAME [-f FORMAT]")); + + /* Parse arguments starting at the second one. */ + format = FORMAT_NATURAL; + formatFound = 0; + optind = 1; + 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")); + + format = mi_parse_format (optarg); + formatFound = 1; + break; + } + } + + if (optind < argc) + 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]); 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; } ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-01-24 21:36 ` Marc Khouzam @ 2008-01-25 9:59 ` Eli Zaretskii 2008-01-29 21:45 ` Daniel Jacobowitz 1 sibling, 0 replies; 33+ messages in thread From: Eli Zaretskii @ 2008-01-25 9:59 UTC (permalink / raw) To: Marc Khouzam; +Cc: nickrob, drow, gdb-patches > Date: Thu, 24 Jan 2008 16:35:37 -0500 > From: "Marc Khouzam" <marc.khouzam@ericsson.com> > Cc: "Daniel Jacobowitz" <drow@false.org>, <gdb-patches@sourceware.org> > > New version based on comments. > This patch also includes tests and documentation update. > I ran regression and got no failure in the new tests > that I added. > > Comments? Thanks. The gdb.texinfo part of your patch is approved. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-01-24 21:36 ` Marc Khouzam 2008-01-25 9:59 ` Eli Zaretskii @ 2008-01-29 21:45 ` Daniel Jacobowitz 2008-02-01 19:23 ` Marc Khouzam 1 sibling, 1 reply; 33+ messages in thread From: Daniel Jacobowitz @ 2008-01-29 21:45 UTC (permalink / raw) To: Marc Khouzam; +Cc: Nick Roberts, gdb-patches On Thu, Jan 24, 2008 at 04:35:37PM -0500, Marc Khouzam wrote: > New version based on comments. > This patch also includes tests and documentation update. > I ran regression and got no failure in the new tests > that I added. > > Comments? This looks basically OK to me. Just minor comments. > +extern char *varobj_get_formatted_value (struct varobj *var, > + enum varobj_display_formats format); You've almost got GNU indentation style, but not quite. We replace leading eight space sequences with tabs, and parameters should line up. Of course that doesn't quite fit here, so some rule has to bend; it doesn't really matter which, but please use the tabs. > +struct cleanup_set_format_struct > +{ > + struct varobj *var; > + enum varobj_display_formats format; > +}; Two space indent there. > + varobj_set_display_format (cleanup_struct->var, > + cleanup_struct->format); These don't line up - I suspect a different tab width somewhere along the line? > Index: gdb/doc/gdb.texinfo > =================================================================== > RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v > retrieving revision 1.459 > diff -u -r1.459 gdb.texinfo > --- gdb/doc/gdb.texinfo 23 Jan 2008 11:26:29 -0000 1.459 > +++ gdb/doc/gdb.texinfo 24 Jan 2008 21:26:46 -0000 > @@ -19995,12 +19995,16 @@ > @subsubheading Synopsis > > @smallexample > - -var-evaluate-expression @var{name} > + -var-evaluate-expression @var{name} [-f @var{format-spec}] > @end smallexample The grammar for MI commands says options always come first. They're optional, anyway, so how about -var-evaluate-expression [-f @var{format-spec}] @var{name}? > Index: gdb/mi/mi-cmd-var.c > =================================================================== > RCS file: /cvs/src/src/gdb/mi/mi-cmd-var.c,v > retrieving revision 1.44 > diff -u -r1.44 mi-cmd-var.c > --- gdb/mi/mi-cmd-var.c 23 Jan 2008 06:13:44 -0000 1.44 > +++ gdb/mi/mi-cmd-var.c 24 Jan 2008 21:26:46 -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"; Yes, we do live in the distant past. If you add a #include, you have to manually update gdb/Makefile.in. Maybe this year we'll get automatic dependencies. > + switch ((enum opt) opt) > + { > + case OP_FORMAT: > + if (formatFound) > + error (_("mi_cmd_var_evaluate_expression: cannot specify format more than once")); > + > + format = mi_parse_format (optarg); > + formatFound = 1; > + break; > + } Funny indentation again. -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-01-29 21:45 ` Daniel Jacobowitz @ 2008-02-01 19:23 ` Marc Khouzam 2008-02-02 10:31 ` Eli Zaretskii ` (2 more replies) 0 siblings, 3 replies; 33+ messages in thread From: Marc Khouzam @ 2008-02-01 19:23 UTC (permalink / raw) To: Daniel Jacobowitz; +Cc: Nick Roberts, gdb-patches Sorry for the delay. Thanks for the comments. > We replace leading eight space sequences with tabs, and > parameters should line up. I have fixed this and fixed a couple of other spots that were not following that rule in the two or three files I affected. > The grammar for MI commands says options always come first. They're > optional, anyway, so how about -var-evaluate-expression [-f > @var{format-spec}] @var{name}? Yes, I like that better too. I have updated the code, the tests and the doc to follow this format. Maybe Eli wants to re-approve? > Yes, we do live in the distant past. If you add a #include, you have > to manually update gdb/Makefile.in. Good you noticed. Fixed. Here is the revised patch. I rebuilt (not the doc) and ran regression. Marc === ### Eclipse Workspace Patch 1.0 #P src Index: gdb/testsuite/gdb.mi/mi-var-display.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-var-display.exp,v retrieving revision 1.21 diff -u -r1.21 mi-var-display.exp --- gdb/testsuite/gdb.mi/mi-var-display.exp 23 Jan 2008 06:20:57 -0000 1.21 +++ gdb/testsuite/gdb.mi/mi-var-display.exp 1 Feb 2008 19:12:01 -0000 @@ -171,6 +171,49 @@ mi_gdb_test "-var-evaluate-expression foo" \ "\\^done,value=\"3\"" \ "eval variable foo" + +# Test: c_variable-6.19 +# Desc: check optional format parameter of var-evaluate-expression +# and check that current format is not changed +mi_gdb_test "-var-evaluate-expression -f hex foo" \ + "\\^done,value=\"0x3\"" \ + "eval variable foo in hex" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in hex" + +mi_gdb_test "-var-evaluate-expression -f octal foo" \ + "\\^done,value=\"03\"" \ + "eval variable foo in octal" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in octal" + +mi_gdb_test "-var-evaluate-expression -f decimal foo" \ + "\\^done,value=\"3\"" \ + "eval variable foo in decimal" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in decimal" + +mi_gdb_test "-var-evaluate-expression -f nat foo" \ + "\\^done,value=\"0x3\"" \ + "eval variable foo in natural" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in natural" + +mi_gdb_test "-var-evaluate-expression -f bin foo" \ + "\\^done,value=\"11\"" \ + "eval variable foo in binary" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in binary" mi_gdb_test "-var-delete foo" \ "\\^done,ndeleted=\"1\"" \ Index: gdb/testsuite/gdb.mi/mi2-var-display.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi2-var-display.exp,v retrieving revision 1.14 diff -u -r1.14 mi2-var-display.exp --- gdb/testsuite/gdb.mi/mi2-var-display.exp 23 Jan 2008 21:05:16 -0000 1.14 +++ gdb/testsuite/gdb.mi/mi2-var-display.exp 1 Feb 2008 19:12:01 -0000 @@ -171,6 +171,49 @@ mi_gdb_test "-var-evaluate-expression foo" \ "\\^done,value=\"3\"" \ "eval variable foo" + +# Test: c_variable-6.19 +# Desc: check optional format parameter of var-evaluate-expression +# and check that current format is not changed +mi_gdb_test "-var-evaluate-expression -f hex foo" \ + "\\^done,value=\"0x3\"" \ + "eval variable foo in hex" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in hex" + +mi_gdb_test "-var-evaluate-expression -f octal foo" \ + "\\^done,value=\"03\"" \ + "eval variable foo in octal" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in octal" + +mi_gdb_test "-var-evaluate-expression -f decimal foo" \ + "\\^done,value=\"3\"" \ + "eval variable foo in decimal" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in decimal" + +mi_gdb_test "-var-evaluate-expression -f nat foo" \ + "\\^done,value=\"0x3\"" \ + "eval variable foo in natural" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in natural" + +mi_gdb_test "-var-evaluate-expression -f bin foo" \ + "\\^done,value=\"11\"" \ + "eval variable foo in binary" + +mi_gdb_test "-var-show-format foo" \ + "\\^done,format=\"decimal\"" \ + "show format variable foo after eval in binary" mi_gdb_test "-var-delete foo" \ "\\^done,ndeleted=\"1\"" \ Index: gdb/varobj.h =================================================================== RCS file: /cvs/src/src/gdb/varobj.h,v retrieving revision 1.15 diff -u -r1.15 varobj.h --- gdb/varobj.h 30 Jan 2008 07:17:31 -0000 1.15 +++ gdb/varobj.h 1 Feb 2008 19:11:56 -0000 @@ -109,6 +109,9 @@ extern int varobj_get_attributes (struct varobj *var); +extern char *varobj_get_formatted_value (struct varobj *var, + enum varobj_display_formats format); + extern char *varobj_get_value (struct varobj *var); extern int varobj_set_value (struct varobj *var, char *expression); Index: gdb/varobj.c =================================================================== RCS file: /cvs/src/src/gdb/varobj.c,v retrieving revision 1.101 diff -u -r1.101 varobj.c --- gdb/varobj.c 30 Jan 2008 07:17:31 -0000 1.101 +++ gdb/varobj.c 1 Feb 2008 19:11:56 -0000 @@ -188,6 +188,9 @@ static struct cleanup *make_cleanup_free_variable (struct varobj *var); +static struct cleanup *make_cleanup_set_format (struct varobj *var, + enum varobj_display_formats format); + static struct type *get_type (struct varobj *var); static struct type *get_value_type (struct varobj *var); @@ -831,6 +834,24 @@ return my_value_of_variable (var); } +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); + + 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) /* 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)) { /* If type_changed is 1, install_new_value will never return non-zero, so we'll never report the same variable twice. */ @@ -1520,6 +1541,36 @@ return make_cleanup (do_free_variable_cleanup, var); } +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, + cleanup_struct->format); + + xfree (cleanup_struct); +} + +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. @@ -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) + { + 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); } else { ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-01 19:23 ` Marc Khouzam @ 2008-02-02 10:31 ` Eli Zaretskii 2008-02-03 19:43 ` Marc Khouzam 2008-02-02 11:20 ` Vladimir Prus 2008-02-03 21:46 ` Daniel Jacobowitz 2 siblings, 1 reply; 33+ messages in thread From: Eli Zaretskii @ 2008-02-02 10:31 UTC (permalink / raw) To: Marc Khouzam; +Cc: drow, nickrob, gdb-patches > Date: Fri, 1 Feb 2008 14:23:19 -0500 > From: "Marc Khouzam" <marc.khouzam@ericsson.com> > Cc: "Nick Roberts" <nickrob@snap.net.nz>, <gdb-patches@sourceware.org> > > > The grammar for MI commands says options always come first. They're > > optional, anyway, so how about -var-evaluate-expression [-f > > @var{format-spec}] @var{name}? > > Yes, I like that better too. > I have updated the code, the tests and the doc to follow this format. > Maybe Eli wants to re-approve? If you need my reapproval, you've got it. Thanks. > I rebuilt (not the doc) and ran regression. ^^^^^^^^^^^ Why not? ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-02 10:31 ` Eli Zaretskii @ 2008-02-03 19:43 ` Marc Khouzam 2008-02-03 20:01 ` Eli Zaretskii 0 siblings, 1 reply; 33+ messages in thread From: Marc Khouzam @ 2008-02-03 19:43 UTC (permalink / raw) To: Eli Zaretskii; +Cc: drow, nickrob, gdb-patches > > I rebuilt (not the doc) and ran regression. > ^^^^^^^^^^^ > Why not? Didn't know how... I'm sure it is just a make target, but I tried a couple (like make html) but didn't find the files... ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-03 19:43 ` Marc Khouzam @ 2008-02-03 20:01 ` Eli Zaretskii 2008-02-04 14:17 ` Marc Khouzam 0 siblings, 1 reply; 33+ messages in thread From: Eli Zaretskii @ 2008-02-03 20:01 UTC (permalink / raw) To: Marc Khouzam; +Cc: drow, nickrob, gdb-patches > Date: Sun, 3 Feb 2008 14:40:33 -0500 > From: "Marc Khouzam" <marc.khouzam@ericsson.com> > Cc: <drow@false.org>, <nickrob@snap.net.nz>, <gdb-patches@sourceware.org> > > > > I rebuilt (not the doc) and ran regression. > > ^^^^^^^^^^^ > > Why not? > > Didn't know how... chdir to gdb/doc directory, and then type this: make info If the tree is not configure'd yet, you can do this: make -f Makefile.in srcdir=. info ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-03 20:01 ` Eli Zaretskii @ 2008-02-04 14:17 ` Marc Khouzam 2008-02-04 21:00 ` Eli Zaretskii 0 siblings, 1 reply; 33+ messages in thread From: Marc Khouzam @ 2008-02-04 14:17 UTC (permalink / raw) To: Eli Zaretskii; +Cc: drow, nickrob, gdb-patches Thanks for taking the time to follow up. Once I built, I found a compile problem with the documentation that I changed! It was missing an @anchor for var-set-format Here is the latest part of the patch that is for the documentation only: ### Eclipse Workspace Patch 1.0 #P src 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 4 Feb 2008 14:15:47 -0000 @@ -19919,6 +19919,7 @@ Sets the output format for the value of the object @var{name} to be @var{format-spec}. +@anchor{-var-set-format} The syntax for the @var{format-spec} is as follows: @smallexample @@ -20095,12 +20096,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 +20158,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. -----Original Message----- From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org]On Behalf Of Eli Zaretskii Sent: Sunday, February 03, 2008 3:00 PM To: Marc Khouzam Cc: drow@false.org; nickrob@snap.net.nz; gdb-patches@sourceware.org Subject: Re: [Patch] -var-evaluate-expression NAME [FORMAT] > Date: Sun, 3 Feb 2008 14:40:33 -0500 > From: "Marc Khouzam" <marc.khouzam@ericsson.com> > Cc: <drow@false.org>, <nickrob@snap.net.nz>, <gdb-patches@sourceware.org> > > > > I rebuilt (not the doc) and ran regression. > > ^^^^^^^^^^^ > > Why not? > > Didn't know how... chdir to gdb/doc directory, and then type this: make info If the tree is not configure'd yet, you can do this: make -f Makefile.in srcdir=. info ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-04 14:17 ` Marc Khouzam @ 2008-02-04 21:00 ` Eli Zaretskii 0 siblings, 0 replies; 33+ messages in thread From: Eli Zaretskii @ 2008-02-04 21:00 UTC (permalink / raw) To: Marc Khouzam; +Cc: drow, nickrob, gdb-patches > Date: Mon, 4 Feb 2008 09:16:48 -0500 > From: "Marc Khouzam" <marc.khouzam@ericsson.com> > Cc: <drow@false.org>, <nickrob@snap.net.nz>, <gdb-patches@sourceware.org> > > Thanks for taking the time to follow up. > Once I built, I found a compile problem with the documentation > that I changed! It was missing an @anchor for var-set-format > Here is the latest part of the patch that is for the documentation only: Thanks, this is fine. ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-01 19:23 ` Marc Khouzam 2008-02-02 10:31 ` Eli Zaretskii @ 2008-02-02 11:20 ` Vladimir Prus 2008-02-03 19:55 ` Marc Khouzam 2008-02-03 21:46 ` Daniel Jacobowitz 2 siblings, 1 reply; 33+ messages in thread From: Vladimir Prus @ 2008-02-02 11:20 UTC (permalink / raw) To: Marc Khouzam, gdb-patches 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-02 11:20 ` Vladimir Prus @ 2008-02-03 19:55 ` Marc Khouzam 2008-02-03 20:42 ` Joel Brobecker 0 siblings, 1 reply; 33+ messages in thread From: Marc Khouzam @ 2008-02-03 19:55 UTC (permalink / raw) To: Vladimir Prus, gdb-patches > I honestly don't like this 'set global format, then cleanup' > trick, but that's something we can fix later. There was another suggested solution in: http://sourceware.org/ml/gdb-patches/2008-01/msg00531.html But it seemed like too much impact. > > - else if (var->value == NULL && value == NULL) > > + else if (var->value == NULL && value == NULL) > > Did you do something about indentation here? I went through the file and replaced any starting sequence of 8 spaces with a tab. I hesitated at changing parts of the file I didn't actually change, but since they were minor changes, I though it would make the file more readable. [...] > > if (mi_version (uiout) > 1) > > - do_cleanups (cleanup); > > + do_cleanups (cleanup); > > Lots of reformatting here as well. The patch looks otherwise > good to me. All these reformats are replacing 8 spaces with tabs. Unless you guys want me to revert that extra reformatting, I guess the patch is good? Who's gonna apply it? :-) Thanks Marc ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-03 19:55 ` Marc Khouzam @ 2008-02-03 20:42 ` Joel Brobecker 2008-02-03 21:41 ` Daniel Jacobowitz 0 siblings, 1 reply; 33+ messages in thread From: Joel Brobecker @ 2008-02-03 20:42 UTC (permalink / raw) To: Marc Khouzam; +Cc: Vladimir Prus, gdb-patches > I went through the file and replaced any starting sequence of > 8 spaces with a tab. I hesitated at changing parts of the file > I didn't actually change, but since they were minor changes, > I though it would make the file more readable. We usually prefer it if you do the reformatting in a separate patch. It makes it easier to review the non-reformatting bits by not drowning the meat of your patch with other, less critical, hunks. -- Joel ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-03 20:42 ` Joel Brobecker @ 2008-02-03 21:41 ` Daniel Jacobowitz 0 siblings, 0 replies; 33+ messages in thread From: Daniel Jacobowitz @ 2008-02-03 21:41 UTC (permalink / raw) To: Joel Brobecker; +Cc: Marc Khouzam, Vladimir Prus, gdb-patches On Sun, Feb 03, 2008 at 12:42:12PM -0800, Joel Brobecker wrote: > We usually prefer it if you do the reformatting in a separate patch. > It makes it easier to review the non-reformatting bits by not drowning > the meat of your patch with other, less critical, hunks. ... But now that it's done it's not worth undoing, IMO. -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-01 19:23 ` Marc Khouzam 2008-02-02 10:31 ` Eli Zaretskii 2008-02-02 11:20 ` Vladimir Prus @ 2008-02-03 21:46 ` Daniel Jacobowitz 2008-02-04 17:00 ` Marc Khouzam ` (2 more replies) 2 siblings, 3 replies; 33+ messages in thread From: Daniel Jacobowitz @ 2008-02-03 21:46 UTC (permalink / raw) To: Marc Khouzam; +Cc: Nick Roberts, gdb-patches On Fri, Feb 01, 2008 at 02:23:19PM -0500, Marc Khouzam wrote: > Here is the revised patch. This looks fine. All it needs, to be checked in, is a ChangeLog entry. Could you give that a try? As for how it gets checked in, I can take care of that; or if you would like write-after-approval access to the repository, please fill out the form from the front page of sourceware.org and list me as approving it. Except for one small problem :-( We seem to have skipped a step. Do you have a copyright assignment in place? I can't find a record of it. -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-03 21:46 ` Daniel Jacobowitz @ 2008-02-04 17:00 ` Marc Khouzam 2008-02-04 17:02 ` Marc Khouzam 2008-02-04 17:11 ` Marc Khouzam 2 siblings, 0 replies; 33+ messages in thread From: Marc Khouzam @ 2008-02-04 17:00 UTC (permalink / raw) To: Daniel Jacobowitz; +Cc: Nick Roberts, gdb-patches > > Here is the revised patch. My patch no longer works with the patch Vladimir applied http://sourceware.org/ml/gdb/2008-02/msg00006.html The reason is that Vladimir's patch has c_value_of_variable() always use the stored print_value which is formatted based on the current display format (and not the one specified as an option to -var-evaluate-expression.) I think that is a good way to do it. One solution is to revert c_value_of_variable: - return xstrdup (var->print_value); + return value_get_print_value (var->value, var->format); Although this is very easy, personally, I don't like that too much. Another option is to not only temporarily set var->format but also var->print_value (by calling varobj_set I think a solution based on http://sourceware.org/ml/gdb-patches/2008-01/msg00531.html > > This looks fine. All it needs, to be checked in, is a ChangeLog > entry. Could you give that a try? I updated gdb/ChangeLog, gdb/testsuite/ChangeLog and gdb/doc/ChangeLog > As for how it gets checked in, I can take care of that; or if you > would like write-after-approval access to the repository, please fill > out the form from the front page of sourceware.org and list me as > approving it. Thanks! I filled the form. I gather you should get it soon. > > Except for one small problem :-( We seem to have skipped a step. Do > you have a copyright assignment in place? I can't find a record of > it. ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-03 21:46 ` Daniel Jacobowitz 2008-02-04 17:00 ` Marc Khouzam @ 2008-02-04 17:02 ` Marc Khouzam 2008-02-04 21:40 ` Nick Roberts 2008-02-04 17:11 ` Marc Khouzam 2 siblings, 1 reply; 33+ messages in thread From: Marc Khouzam @ 2008-02-04 17:02 UTC (permalink / raw) To: Daniel Jacobowitz; +Cc: Nick Roberts, gdb-patches Sorry, the below got away from me before I finished. == > > Here is the revised patch. My patch no longer works with the patch Vladimir applied http://sourceware.org/ml/gdb/2008-02/msg00006.html The reason is that Vladimir's patch has c_value_of_variable() always use the stored print_value which is formatted based on the current display format (and not the one specified as an option to -var-evaluate-expression.) I think that is a good way to do it. One solution is to revert c_value_of_variable: - return xstrdup (var->print_value); + return value_get_print_value (var->value, var->format); Although this is very easy, personally, I don't like that too much. Another option is to not only temporarily set var->format but also var->print_value (by calling varobj_set_display_format). I like this even less. I think a solution based on http://sourceware.org/ml/gdb-patches/2008-01/msg00531.html is probably the cleanest. Can I have an educated opinion on this decision? > > This looks fine. All it needs, to be checked in, is a ChangeLog > entry. Could you give that a try? I updated gdb/ChangeLog, gdb/testsuite/ChangeLog and gdb/doc/ChangeLog > As for how it gets checked in, I can take care of that; or if you > would like write-after-approval access to the repository, please fill > out the form from the front page of sourceware.org and list me as > approving it. Thanks! I filled the form. I gather you should get it soon. > > Except for one small problem :-( We seem to have skipped a step. Do > you have a copyright assignment in place? I can't find a record of > it. What is this? Marc ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-04 17:02 ` Marc Khouzam @ 2008-02-04 21:40 ` Nick Roberts 2008-02-04 21:53 ` Nick Roberts 2008-02-05 14:47 ` Marc Khouzam 0 siblings, 2 replies; 33+ messages in thread From: Nick Roberts @ 2008-02-04 21:40 UTC (permalink / raw) To: Marc Khouzam; +Cc: Daniel Jacobowitz, gdb-patches > My patch no longer works with the patch Vladimir applied > http://sourceware.org/ml/gdb/2008-02/msg00006.html > The reason is that Vladimir's patch has c_value_of_variable() > always use the stored print_value which is formatted based on the > current display format (and not the one specified as an option > to -var-evaluate-expression.) I think that is a good way to do it. > > One solution is to revert c_value_of_variable: > - return xstrdup (var->print_value); > + return value_get_print_value (var->value, var->format); > > Although this is very easy, personally, I don't like that too much. > > Another option is to not only temporarily set var->format > but also var->print_value (by calling varobj_set_display_format). > I like this even less. > > I think a solution based on > http://sourceware.org/ml/gdb-patches/2008-01/msg00531.html > is probably the cleanest. Can I have an educated opinion on > this decision? This also uses value_get_print_value in c_value_of_variable. I can't seem to apply your earlier patch as I get spurious characters when I save it (= -> =3D, \n -> =20\n, etc) but I wonder if we've made things too complicated. What happens if you just use value_get_print_value (var->value, var->format) directly in mi_cmd_var_evaluate_expression instead of varobj_get_value (var)? -- Nick http://www.inet.net.nz/~nickrob ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-04 21:40 ` Nick Roberts @ 2008-02-04 21:53 ` Nick Roberts 2008-02-05 14:47 ` Marc Khouzam 1 sibling, 0 replies; 33+ messages in thread From: Nick Roberts @ 2008-02-04 21:53 UTC (permalink / raw) To: Marc Khouzam, Daniel Jacobowitz, gdb-patches > What happens if you just use value_get_print_value (var->value, var->format) > directly in mi_cmd_var_evaluate_expression instead of varobj_get_value (var)? I mean value_get_print_value (var->value, format) -- Nick http://www.inet.net.nz/~nickrob ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-04 21:40 ` Nick Roberts 2008-02-04 21:53 ` Nick Roberts @ 2008-02-05 14:47 ` Marc Khouzam 2008-02-05 20:43 ` Nick Roberts 1 sibling, 1 reply; 33+ messages in thread From: Marc Khouzam @ 2008-02-05 14:47 UTC (permalink / raw) To: Nick Roberts; +Cc: Daniel Jacobowitz, gdb-patches > > I think a solution based on > > http://sourceware.org/ml/gdb-patches/2008-01/msg00531.html > > is probably the cleanest. Can I have an educated opinion on > > this decision? > > This also uses value_get_print_value in c_value_of_variable. Did you mean that this solution "also uses print_value" after Vladimir's patch? What we would do is use the format parameter that would be passed all the way to c_value_of_variable; if the specified format is the same as var->format we call print_value (as done in Vladimir's patch), but if the format is something different, we call value_get_print_value (var->value, format). (See below). > I can't seem to apply your earlier patch as I get spurious characters > when I save it (= -> =3D, \n -> =20\n, etc) but I wonder if we've made > things too complicated. I've redone the patch (slightly cleaner than before). It's at the bottom. > What happens if you just use value_get_print_value (var->value, var->format) > directly in mi_cmd_var_evaluate_expression instead of varobj_get_value (var)? This won't work, because it bypassed all the logic done between the call to varobj_get_value() and the one to value_get_print_value(); such things as returning "{...}" for structs and unions, returning "[numChildren]" for arrays; it would also bypass the checks for var->value == NULL, and value_lazy(), etc. Marc == Indentation is not correct, tests and doc have been removed. This is just for discussion since I have to wait for my copyright assignment anyway. I did test and run regression successfully. Index: gdb/varobj.h =================================================================== RCS file: /cvs/src/src/gdb/varobj.h,v retrieving revision 1.15 diff -u -r1.15 varobj.h --- gdb/varobj.h 30 Jan 2008 07:17:31 -0000 1.15 +++ gdb/varobj.h 5 Feb 2008 14:37:03 -0000 @@ -109,6 +109,9 @@ extern int varobj_get_attributes (struct varobj *var); +extern char *varobj_get_formatted_value (struct varobj *var, +enum varobj_display_formats format); + extern char *varobj_get_value (struct varobj *var); extern int varobj_set_value (struct varobj *var, char *expression); Index: gdb/varobj.c =================================================================== RCS file: /cvs/src/src/gdb/varobj.c,v retrieving revision 1.103 diff -u -r1.103 varobj.c --- gdb/varobj.c 4 Feb 2008 07:49:04 -0000 1.103 +++ gdb/varobj.c 5 Feb 2008 14:37:03 -0000 @@ -217,7 +217,8 @@ static struct value *value_of_child (struct varobj *parent, int index); -static char *my_value_of_variable (struct varobj *var); +static char *my_value_of_variable (struct varobj *var, + enum varobj_display_formats format); static char *value_get_print_value (struct value *value, enum varobj_display_formats format); @@ -242,7 +243,8 @@ static struct type *c_type_of_child (struct varobj *parent, int index); -static char *c_value_of_variable (struct varobj *var); +static char *c_value_of_variable (struct varobj *var, + enum varobj_display_formats format); /* C++ implementation */ @@ -262,7 +264,8 @@ static struct type *cplus_type_of_child (struct varobj *parent, int index); -static char *cplus_value_of_variable (struct varobj *var); +static char *cplus_value_of_variable (struct varobj *var, + enum varobj_display_formats format); /* Java implementation */ @@ -280,7 +283,8 @@ static struct type *java_type_of_child (struct varobj *parent, int index); -static char *java_value_of_variable (struct varobj *var); +static char *java_value_of_variable (struct varobj *var, + enum varobj_display_formats format); /* The language specific vector */ @@ -313,7 +317,8 @@ struct type *(*type_of_child) (struct varobj * parent, int index); /* The current value of VAR. */ - char *(*value_of_variable) (struct varobj * var); + char *(*value_of_variable) (struct varobj * var, + enum varobj_display_formats format); }; /* Array of known source language routines. */ @@ -833,9 +838,16 @@ } char * +varobj_get_formatted_value (struct varobj *var, + enum varobj_display_formats format) +{ + return my_value_of_variable (var, format); +} + +char * varobj_get_value (struct varobj *var) { - return my_value_of_variable (var); + return my_value_of_variable (var, var->format); } /* Set the value of an object variable (if it is editable) to the @@ -1770,10 +1782,11 @@ /* GDB already has a command called "value_of_variable". Sigh. */ static char * -my_value_of_variable (struct varobj *var) +my_value_of_variable (struct varobj *var, + enum varobj_display_formats format) { if (var->root->is_valid) - return (*var->root->lang->value_of_variable) (var); + return (*var->root->lang->value_of_variable) (var, format); else return NULL; } @@ -2207,7 +2220,8 @@ } static char * -c_value_of_variable (struct varobj *var) +c_value_of_variable (struct varobj *var, + enum varobj_display_formats format) { /* BOGUS: if val_print sees a struct/class, or a reference to one, it will print out its children instead of "{...}". So we need to @@ -2252,7 +2266,13 @@ gdb_assert (varobj_value_is_changeable_p (var)); gdb_assert (!value_lazy (var->value)); - return xstrdup (var->print_value); + + /* If the specified format is the current one, + we can reuse print_value */ + if (format == var->format) + return xstrdup (var->print_value); + else + return value_get_print_value (var->value, format); } } } @@ -2578,7 +2598,8 @@ } static char * -cplus_value_of_variable (struct varobj *var) +cplus_value_of_variable (struct varobj *var, + enum varobj_display_formats format) { /* If we have one of our special types, don't print out @@ -2586,7 +2607,7 @@ if (CPLUS_FAKE_CHILD (var)) return xstrdup (""); - return c_value_of_variable (var); + return c_value_of_variable (var, format); } /* Java */ @@ -2661,9 +2682,10 @@ } static char * -java_value_of_variable (struct varobj *var) +java_value_of_variable (struct varobj *var, + enum varobj_display_formats format) { - return cplus_value_of_variable (var); + return cplus_value_of_variable (var, format); } extern void _initialize_varobj (void); 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 5 Feb 2008 14:37:03 -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/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 5 Feb 2008 14:37:09 -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); } else { ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-05 14:47 ` Marc Khouzam @ 2008-02-05 20:43 ` Nick Roberts 2008-02-05 20:55 ` Marc Khouzam 0 siblings, 1 reply; 33+ messages in thread From: Nick Roberts @ 2008-02-05 20:43 UTC (permalink / raw) To: Marc Khouzam; +Cc: Daniel Jacobowitz, gdb-patches > > What happens if you just use value_get_print_value (var->value, > > var->format) directly in mi_cmd_var_evaluate_expression instead of > > varobj_get_value (var)? > > This won't work, because it bypassed all the logic done between the call to > varobj_get_value() and the one to value_get_print_value(); such things as > returning "{...}" for structs and unions, returning "[numChildren]" for > arrays; it would also bypass the checks for var->value == NULL, and > value_lazy(), etc. I didn't mean use value_get_print_value in all cases but something like: if (formatFound && varobj_value_is_changeable_p (var)) ui_out_field_string (uiout, "value", value_get_print_value (varobj_get_struct_value (var), format)); else ui_out_field_string (uiout, "value", varobj_get_value (var)); where varobj_get_struct_value is defined in varobj.c as: struct value * varobj_get_struct_value (struct varobj *var) { return var->value; } I've not checked all the details and it might need tweaking. If var->value == NULL, I think you just get a null string: -var-evaluate-expression var1 ^done,value="" If varobj_value_is_changeable_p is t and the value not NULL, I don't think the value can be lazy etc. -- Nick http://www.inet.net.nz/~nickrob ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-05 20:43 ` Nick Roberts @ 2008-02-05 20:55 ` Marc Khouzam 2008-02-05 21:18 ` Nick Roberts 0 siblings, 1 reply; 33+ messages in thread From: Marc Khouzam @ 2008-02-05 20:55 UTC (permalink / raw) To: Nick Roberts; +Cc: Daniel Jacobowitz, gdb-patches You don't like my new patch? :-) I feels less risky (and it's cleaner than my orginal one.) My hesitation with your suggestion is that it seems more risky to forget a case, and that it is not as future-proof because it duplicates some logic in mi-var-cmd and varobj. Marc -----Original Message----- From: Nick Roberts [mailto:nickrob@snap.net.nz] Sent: Tuesday, February 05, 2008 3:43 PM To: Marc Khouzam Cc: Daniel Jacobowitz; gdb-patches@sourceware.org Subject: RE: [Patch] -var-evaluate-expression NAME [FORMAT] > > What happens if you just use value_get_print_value (var->value, > > var->format) directly in mi_cmd_var_evaluate_expression instead of > > varobj_get_value (var)? > > This won't work, because it bypassed all the logic done between the call to > varobj_get_value() and the one to value_get_print_value(); such things as > returning "{...}" for structs and unions, returning "[numChildren]" for > arrays; it would also bypass the checks for var->value == NULL, and > value_lazy(), etc. I didn't mean use value_get_print_value in all cases but something like: if (formatFound && varobj_value_is_changeable_p (var)) ui_out_field_string (uiout, "value", value_get_print_value (varobj_get_struct_value (var), format)); else ui_out_field_string (uiout, "value", varobj_get_value (var)); where varobj_get_struct_value is defined in varobj.c as: struct value * varobj_get_struct_value (struct varobj *var) { return var->value; } I've not checked all the details and it might need tweaking. If var->value == NULL, I think you just get a null string: -var-evaluate-expression var1 ^done,value="" If varobj_value_is_changeable_p is t and the value not NULL, I don't think the value can be lazy etc. -- Nick http://www.inet.net.nz/~nickrob ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-05 20:55 ` Marc Khouzam @ 2008-02-05 21:18 ` Nick Roberts 0 siblings, 0 replies; 33+ messages in thread From: Nick Roberts @ 2008-02-05 21:18 UTC (permalink / raw) To: Marc Khouzam; +Cc: Daniel Jacobowitz, gdb-patches > You don't like my new patch? :-) > I feels less risky (and it's cleaner than > my orginal one.) Well it's probably smaller than it looks as there are many formatting changes. "Don't like" is probably too strong, it just seems a bit intrusive for a feature that only you may use. However, I don't object to your patch,... > My hesitation with your suggestion is that it > seems more risky to forget a case, and that it > is not as future-proof because it duplicates > some logic in mi-var-cmd and varobj. ...and it probably is safer. -- Nick http://www.inet.net.nz/~nickrob ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-02-03 21:46 ` Daniel Jacobowitz 2008-02-04 17:00 ` Marc Khouzam 2008-02-04 17:02 ` Marc Khouzam @ 2008-02-04 17:11 ` Marc Khouzam 2 siblings, 0 replies; 33+ messages in thread From: Marc Khouzam @ 2008-02-04 17:11 UTC (permalink / raw) To: Daniel Jacobowitz; +Cc: Nick Roberts, gdb-patches > Except for one small problem :-( We seem to have skipped a step. Do > you have a copyright assignment in place? I can't find a record of it. Ok, I looked this up. Let me get back to you on this. Marc ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-01-22 20:38 [Patch] -var-evaluate-expression NAME [FORMAT] Marc Khouzam 2008-01-23 3:04 ` Nick Roberts @ 2008-01-23 3:48 ` Daniel Jacobowitz 2008-01-23 14:44 ` Marc Khouzam 1 sibling, 1 reply; 33+ messages in thread From: Daniel Jacobowitz @ 2008-01-23 3:48 UTC (permalink / raw) To: Marc Khouzam; +Cc: gdb-patches On Tue, Jan 22, 2008 at 03:30:43PM -0500, Marc Khouzam wrote: > Hi, > > Here is a patch to add an optional format parameter to -var-evaluate-expression. > -var-evaluate-expression NAME [FORMAT] Other MI commands seem to take arguments, so would [-f FORMAT] be better? -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-01-23 3:48 ` Daniel Jacobowitz @ 2008-01-23 14:44 ` Marc Khouzam 0 siblings, 0 replies; 33+ messages in thread From: Marc Khouzam @ 2008-01-23 14:44 UTC (permalink / raw) To: Daniel Jacobowitz; +Cc: gdb-patches > Other MI commands seem to take arguments, so would [-f FORMAT] be > better? No objects from me to -var-evaluate-expression NAME [-f FORMAT] ^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [Patch] -var-evaluate-expression NAME [FORMAT]
@ 2008-01-28 14:41 Marc Khouzam
2008-01-28 14:48 ` Daniel Jacobowitz
0 siblings, 1 reply; 33+ messages in thread
From: Marc Khouzam @ 2008-01-28 14:41 UTC (permalink / raw)
To: gdb-patches
Hi,
This is my first suggested patch, so I'm not sure if I have followed
the process. What exactly happens now about this patch?
Is it approved? And if so, should it be committed? And by whom?
http://sourceware.org/ml/gdb-patches/2008-01/msg00592.html
Thanks
Marc
>> Date: Thu, 24 Jan 2008 16:35:37 -0500
>> From: "Marc Khouzam" <marc.khouzam@ericsson.com>
>> Cc: "Daniel Jacobowitz" <drow@false.org>, <gdb-patches@sourceware.org>
>>
>> New version based on comments.
>> This patch also includes tests and documentation update.
>> I ran regression and got no failure in the new tests
>> that I added.
>>
>> Comments?
>
>Thanks. The gdb.texinfo part of your patch is approved.
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Patch] -var-evaluate-expression NAME [FORMAT] 2008-01-28 14:41 Marc Khouzam @ 2008-01-28 14:48 ` Daniel Jacobowitz 0 siblings, 0 replies; 33+ messages in thread From: Daniel Jacobowitz @ 2008-01-28 14:48 UTC (permalink / raw) To: Marc Khouzam; +Cc: gdb-patches On Mon, Jan 28, 2008 at 09:28:35AM -0500, Marc Khouzam wrote: > Hi, > > This is my first suggested patch, so I'm not sure if I have followed > the process. What exactly happens now about this patch? > Is it approved? And if so, should it be committed? And by whom? > > http://sourceware.org/ml/gdb-patches/2008-01/msg00592.html Someone who can approve it needs to look at it. I'll try to do so in the next day or two. -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2008-02-05 21:18 UTC | newest] Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-01-22 20:38 [Patch] -var-evaluate-expression NAME [FORMAT] Marc Khouzam 2008-01-23 3:04 ` Nick Roberts 2008-01-23 14:30 ` Marc Khouzam 2008-01-23 14:42 ` Daniel Jacobowitz 2008-01-23 17:04 ` Marc Khouzam 2008-01-23 22:09 ` Nick Roberts 2008-01-24 21:36 ` Marc Khouzam 2008-01-25 9:59 ` Eli Zaretskii 2008-01-29 21:45 ` Daniel Jacobowitz 2008-02-01 19:23 ` Marc Khouzam 2008-02-02 10:31 ` Eli Zaretskii 2008-02-03 19:43 ` Marc Khouzam 2008-02-03 20:01 ` Eli Zaretskii 2008-02-04 14:17 ` Marc Khouzam 2008-02-04 21:00 ` Eli Zaretskii 2008-02-02 11:20 ` Vladimir Prus 2008-02-03 19:55 ` Marc Khouzam 2008-02-03 20:42 ` Joel Brobecker 2008-02-03 21:41 ` Daniel Jacobowitz 2008-02-03 21:46 ` Daniel Jacobowitz 2008-02-04 17:00 ` Marc Khouzam 2008-02-04 17:02 ` Marc Khouzam 2008-02-04 21:40 ` Nick Roberts 2008-02-04 21:53 ` Nick Roberts 2008-02-05 14:47 ` Marc Khouzam 2008-02-05 20:43 ` Nick Roberts 2008-02-05 20:55 ` Marc Khouzam 2008-02-05 21:18 ` Nick Roberts 2008-02-04 17:11 ` Marc Khouzam 2008-01-23 3:48 ` Daniel Jacobowitz 2008-01-23 14:44 ` Marc Khouzam 2008-01-28 14:41 Marc Khouzam 2008-01-28 14:48 ` Daniel Jacobowitz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox