[PATCH] Remove parse_print_values

[PATCH] Remove parse_print_values

[Yao Qi]

Hi,
I find function mi_parse_values_option and parse_print_values are
identical, so this patch is to remove parse_print_values and make
mi_parse_values_option external.  Is it OK?

gdb:

2013-05-31  Yao Qi  <yao@codesourcery.com>

	* mi/mi-cmd-var.c (mi_no_values, mi_simple_values): Move to
	mi-parse.c.  Make them static.
	(mi_all_values): Likewise.
	(mi_parse_values_option): Move to mi-parse.c.  Make it
	external.
	* mi/mi-cmds.h (mi_no_values, mi_simple_values, mi_all_values):
	Remove the declarations.
	* mi/mi-parse.c: Moved from mi-cmd-var.c.
	* mi/mi-parse.h (mi_parse_values_option): Declare.
	* mi/mi-cmd-stack.c: Include mi-parse.h.
	(parse_print_values): Remove
	(mi_cmd_stack_list_locals): Call mi_parse_values_option instead
	of parse_print_values.
	(mi_cmd_stack_list_args, mi_cmd_stack_list_variables): Likewise.
---
 gdb/mi/mi-cmd-stack.c |   25 ++++---------------------
 gdb/mi/mi-cmd-var.c   |   25 +------------------------
 gdb/mi/mi-cmds.h      |    4 ----
 gdb/mi/mi-parse.c     |   22 ++++++++++++++++++++++
 gdb/mi/mi-parse.h     |    4 ++++
 5 files changed, 31 insertions(+), 49 deletions(-)

diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c
index dd365f3..762ebc9 100644
--- a/gdb/mi/mi-cmd-stack.c
+++ b/gdb/mi/mi-cmd-stack.c
@@ -35,6 +35,7 @@
 #include "mi-getopt.h"
 #include "python/python.h"
 #include <ctype.h>
+#include "mi-parse.h"
 
 enum what_to_list { locals, arguments, all };
 
@@ -200,24 +201,6 @@ mi_cmd_stack_info_depth (char *command, char **argv, int argc)
   ui_out_field_int (current_uiout, "depth", i);
 }
 
-static enum print_values
-parse_print_values (char *name)
-{
-   if (strcmp (name, "0") == 0
-       || strcmp (name, mi_no_values) == 0)
-     return PRINT_NO_VALUES;
-   else if (strcmp (name, "1") == 0
-	    || strcmp (name, mi_all_values) == 0)
-     return PRINT_ALL_VALUES;
-   else if (strcmp (name, "2") == 0
-	    || strcmp (name, mi_simple_values) == 0)
-     return PRINT_SIMPLE_VALUES;
-   else
-     error (_("Unknown value for PRINT_VALUES: must be: \
-0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
-	    mi_no_values, mi_all_values, mi_simple_values);
-}
-
 /* Print a list of the locals for the current frame.  With argument of
    0, print only the names, with argument of 1 print also the
    values.  */
@@ -238,7 +221,7 @@ mi_cmd_stack_list_locals (char *command, char **argv, int argc)
     error (_("-stack-list-locals: Usage: [--no-frame-filters] PRINT_VALUES"));
 
   frame = get_selected_frame (NULL);
-  print_value = parse_print_values (argv[raw_arg]);
+  print_value = mi_parse_values_option (argv[raw_arg]);
 
    if (! raw_arg && frame_filters)
      {
@@ -293,7 +276,7 @@ mi_cmd_stack_list_args (char *command, char **argv, int argc)
       frame_high = -1;
     }
 
-  print_values = parse_print_values (argv[raw_arg]);
+  print_values = mi_parse_values_option (argv[raw_arg]);
 
   /* Let's position fi on the frame at which to start the
      display. Could be the innermost frame if the whole stack needs
@@ -368,7 +351,7 @@ mi_cmd_stack_list_variables (char *command, char **argv, int argc)
 	     "[--no-frame-filters] PRINT_VALUES"));
 
    frame = get_selected_frame (NULL);
-   print_value = parse_print_values (argv[raw_arg]);
+   print_value = mi_parse_values_option (argv[raw_arg]);
 
    if (! raw_arg && frame_filters)
      {
diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index a069346..86c28eb 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -29,10 +29,7 @@
 #include "gdb_string.h"
 #include "mi-getopt.h"
 #include "gdbthread.h"
-
-const char mi_no_values[] = "--no-values";
-const char mi_simple_values[] = "--simple-values";
-const char mi_all_values[] = "--all-values";
+#include "mi-parse.h"
 
 extern unsigned int varobjdebug;		/* defined in varobj.c.  */
 
@@ -340,26 +337,6 @@ mi_cmd_var_info_num_children (char *command, char **argv, int argc)
   ui_out_field_int (uiout, "numchild", varobj_get_num_children (var));
 }
 
-/* Parse a string argument into a print_values value.  */
-
-static enum print_values
-mi_parse_values_option (const char *arg)
-{
-  if (strcmp (arg, "0") == 0
-      || strcmp (arg, mi_no_values) == 0)
-    return PRINT_NO_VALUES;
-  else if (strcmp (arg, "1") == 0
-	   || strcmp (arg, mi_all_values) == 0)
-    return PRINT_ALL_VALUES;
-  else if (strcmp (arg, "2") == 0
-	   || strcmp (arg, mi_simple_values) == 0)
-    return PRINT_SIMPLE_VALUES;
-  else
-    error (_("Unknown value for PRINT_VALUES\n\
-Must be: 0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
-	   mi_no_values, mi_simple_values, mi_all_values);
-}
-
 /* Return 1 if given the argument PRINT_VALUES we should display
    the varobj VAR.  */
 
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index 8199d15..8839319 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -28,10 +28,6 @@ enum print_values {
    PRINT_SIMPLE_VALUES
 };
 
-extern const char mi_no_values[];
-extern const char mi_simple_values[];
-extern const char mi_all_values[];
-
 typedef void (mi_cmd_argv_ftype) (char *command, char **argv, int argc);
 
 /* Declarations of the functions implementing each command.  */
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index 15fb778..69ef7b8 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -28,6 +28,10 @@
 #include "gdb_string.h"
 #include "cli/cli-utils.h"
 
+const static char mi_no_values[] = "--no-values";
+const static char mi_simple_values[] = "--simple-values";
+const static char mi_all_values[] = "--all-values";
+
 /* Like parse_escape, but leave the results as a host char, not a
    target char.  */
 
@@ -373,3 +377,21 @@ mi_parse (const char *cmd, char **token)
   parse->op = MI_COMMAND;
   return parse;
 }
+
+enum print_values
+mi_parse_values_option (const char *name)
+{
+   if (strcmp (name, "0") == 0
+       || strcmp (name, mi_no_values) == 0)
+     return PRINT_NO_VALUES;
+   else if (strcmp (name, "1") == 0
+	    || strcmp (name, mi_all_values) == 0)
+     return PRINT_ALL_VALUES;
+   else if (strcmp (name, "2") == 0
+	    || strcmp (name, mi_simple_values) == 0)
+     return PRINT_SIMPLE_VALUES;
+   else
+     error (_("Unknown value for PRINT_VALUES: must be: \
+0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
+	    mi_no_values, mi_all_values, mi_simple_values);
+}
diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
index 324ae5d..d061b97 100644
--- a/gdb/mi/mi-parse.h
+++ b/gdb/mi/mi-parse.h
@@ -66,4 +66,8 @@ extern struct mi_parse *mi_parse (const char *cmd, char **token);
 
 extern void mi_parse_free (struct mi_parse *cmd);
 
+/* Parse a string argument into a print_values value.  */
+
+enum print_values mi_parse_values_option (const char *name);
+
 #endif
-- 
1.7.7.6

Re: [PATCH] Remove parse_print_values

[Pedro Alves]

On 05/31/2013 03:16 PM, Yao Qi wrote:

> 	* mi/mi-cmds.h (mi_no_values, mi_simple_values, mi_all_values):
> 	Remove the declarations.
> 	* mi/mi-parse.c: Moved from mi-cmd-var.c.

"Moved what?", he asks.  :-)

 	* mi/mi-parse.c (XXX, YYY, ZZZ): Moved from mi-cmd-var.c.

>    frame = get_selected_frame (NULL);
> -  print_value = parse_print_values (argv[raw_arg]);
> +  print_value = mi_parse_values_option (argv[raw_arg]);

Sorry to be picky, but the "mi_parse_values_option" naming to me
has a weaker connection with "enum print_values" than "parse_PRINT_VALUES".
I'd suggest calling the new shared function "mi_parse_print_values".
WDYT?  This is fine with me with that change.

-- 
Pedro Alves

Re: [PATCH] Remove parse_print_values

[Yao Qi]

On 05/31/2013 10:39 PM, Pedro Alves wrote:
> "Moved what?", he asks.:-)
> 
>   	* mi/mi-parse.c (XXX, YYY, ZZZ): Moved from mi-cmd-var.c.
> 

Fixed.

>> >    frame = get_selected_frame (NULL);
>> >-  print_value = parse_print_values (argv[raw_arg]);
>> >+  print_value = mi_parse_values_option (argv[raw_arg]);
> Sorry to be picky, but the "mi_parse_values_option" naming to me
> has a weaker connection with "enum print_values" than "parse_PRINT_VALUES".
> I'd suggest calling the new shared function "mi_parse_print_values".
> WDYT?  This is fine with me with that change.

When I wrote this patch, I thought about keeping parse_print_values or mi_parse_values_option, and I chose mi_parse_values_option finally 
because it has a prefix "mi_".  Since it is a patch to remove
duplication, I don't think further to rename it.
"mi_parse_print_values" is fine with me.

Patch below is what I committed.  Thanks for the review.

-- 
Yao (齐尧)

gdb:

2013-06-03  Yao Qi  <yao@codesourcery.com>

	* mi/mi-cmd-var.c (mi_no_values, mi_simple_values): Move to
	mi-parse.c.  Make them static.
	(mi_all_values): Likewise.
	(mi_parse_values_option): Move to mi-parse.c.  Rename it to
	mi_parse_print_values.  Make it external.
	* mi/mi-cmds.h (mi_no_values, mi_simple_values, mi_all_values):
	Remove the declarations.
	* mi/mi-parse.c (mi_parse_print_values): Moved from mi-cmd-var.c.
	* mi/mi-parse.h (mi_parse_print_values): Declare.
	* mi/mi-cmd-stack.c: Include mi-parse.h.
	(parse_print_values): Remove
	(mi_cmd_stack_list_locals): Call mi_parse_print_values instead
	of parse_print_values.
	(mi_cmd_stack_list_args, mi_cmd_stack_list_variables): Likewise.
---
 gdb/mi/mi-cmd-stack.c |   25 ++++---------------------
 gdb/mi/mi-cmd-var.c   |   29 +++--------------------------
 gdb/mi/mi-cmds.h      |    4 ----
 gdb/mi/mi-parse.c     |   22 ++++++++++++++++++++++
 gdb/mi/mi-parse.h     |    4 ++++
 5 files changed, 33 insertions(+), 51 deletions(-)

diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c
index dd365f3..4b21015 100644
--- a/gdb/mi/mi-cmd-stack.c
+++ b/gdb/mi/mi-cmd-stack.c
@@ -35,6 +35,7 @@
 #include "mi-getopt.h"
 #include "python/python.h"
 #include <ctype.h>
+#include "mi-parse.h"
 
 enum what_to_list { locals, arguments, all };
 
@@ -200,24 +201,6 @@ mi_cmd_stack_info_depth (char *command, char **argv, int argc)
   ui_out_field_int (current_uiout, "depth", i);
 }
 
-static enum print_values
-parse_print_values (char *name)
-{
-   if (strcmp (name, "0") == 0
-       || strcmp (name, mi_no_values) == 0)
-     return PRINT_NO_VALUES;
-   else if (strcmp (name, "1") == 0
-	    || strcmp (name, mi_all_values) == 0)
-     return PRINT_ALL_VALUES;
-   else if (strcmp (name, "2") == 0
-	    || strcmp (name, mi_simple_values) == 0)
-     return PRINT_SIMPLE_VALUES;
-   else
-     error (_("Unknown value for PRINT_VALUES: must be: \
-0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
-	    mi_no_values, mi_all_values, mi_simple_values);
-}
-
 /* Print a list of the locals for the current frame.  With argument of
    0, print only the names, with argument of 1 print also the
    values.  */
@@ -238,7 +221,7 @@ mi_cmd_stack_list_locals (char *command, char **argv, int argc)
     error (_("-stack-list-locals: Usage: [--no-frame-filters] PRINT_VALUES"));
 
   frame = get_selected_frame (NULL);
-  print_value = parse_print_values (argv[raw_arg]);
+  print_value = mi_parse_print_values (argv[raw_arg]);
 
    if (! raw_arg && frame_filters)
      {
@@ -293,7 +276,7 @@ mi_cmd_stack_list_args (char *command, char **argv, int argc)
       frame_high = -1;
     }
 
-  print_values = parse_print_values (argv[raw_arg]);
+  print_values = mi_parse_print_values (argv[raw_arg]);
 
   /* Let's position fi on the frame at which to start the
      display. Could be the innermost frame if the whole stack needs
@@ -368,7 +351,7 @@ mi_cmd_stack_list_variables (char *command, char **argv, int argc)
 	     "[--no-frame-filters] PRINT_VALUES"));
 
    frame = get_selected_frame (NULL);
-   print_value = parse_print_values (argv[raw_arg]);
+   print_value = mi_parse_print_values (argv[raw_arg]);
 
    if (! raw_arg && frame_filters)
      {
diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index a069346..57a2f6b 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -29,10 +29,7 @@
 #include "gdb_string.h"
 #include "mi-getopt.h"
 #include "gdbthread.h"
-
-const char mi_no_values[] = "--no-values";
-const char mi_simple_values[] = "--simple-values";
-const char mi_all_values[] = "--all-values";
+#include "mi-parse.h"
 
 extern unsigned int varobjdebug;		/* defined in varobj.c.  */
 
@@ -340,26 +337,6 @@ mi_cmd_var_info_num_children (char *command, char **argv, int argc)
   ui_out_field_int (uiout, "numchild", varobj_get_num_children (var));
 }
 
-/* Parse a string argument into a print_values value.  */
-
-static enum print_values
-mi_parse_values_option (const char *arg)
-{
-  if (strcmp (arg, "0") == 0
-      || strcmp (arg, mi_no_values) == 0)
-    return PRINT_NO_VALUES;
-  else if (strcmp (arg, "1") == 0
-	   || strcmp (arg, mi_all_values) == 0)
-    return PRINT_ALL_VALUES;
-  else if (strcmp (arg, "2") == 0
-	   || strcmp (arg, mi_simple_values) == 0)
-    return PRINT_SIMPLE_VALUES;
-  else
-    error (_("Unknown value for PRINT_VALUES\n\
-Must be: 0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
-	   mi_no_values, mi_simple_values, mi_all_values);
-}
-
 /* Return 1 if given the argument PRINT_VALUES we should display
    the varobj VAR.  */
 
@@ -428,7 +405,7 @@ mi_cmd_var_list_children (char *command, char **argv, int argc)
   children = varobj_list_children (var, &from, &to);
   ui_out_field_int (uiout, "numchild", to - from);
   if (argc == 2 || argc == 4)
-    print_values = mi_parse_values_option (argv[0]);
+    print_values = mi_parse_print_values (argv[0]);
   else
     print_values = PRINT_NO_VALUES;
 
@@ -698,7 +675,7 @@ mi_cmd_var_update (char *command, char **argv, int argc)
     name = argv[1];
 
   if (argc == 2)
-    print_values = mi_parse_values_option (argv[0]);
+    print_values = mi_parse_print_values (argv[0]);
   else
     print_values = PRINT_NO_VALUES;
 
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index 8199d15..8839319 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -28,10 +28,6 @@ enum print_values {
    PRINT_SIMPLE_VALUES
 };
 
-extern const char mi_no_values[];
-extern const char mi_simple_values[];
-extern const char mi_all_values[];
-
 typedef void (mi_cmd_argv_ftype) (char *command, char **argv, int argc);
 
 /* Declarations of the functions implementing each command.  */
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index 15fb778..0f35f1d 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -28,6 +28,10 @@
 #include "gdb_string.h"
 #include "cli/cli-utils.h"
 
+const static char mi_no_values[] = "--no-values";
+const static char mi_simple_values[] = "--simple-values";
+const static char mi_all_values[] = "--all-values";
+
 /* Like parse_escape, but leave the results as a host char, not a
    target char.  */
 
@@ -373,3 +377,21 @@ mi_parse (const char *cmd, char **token)
   parse->op = MI_COMMAND;
   return parse;
 }
+
+enum print_values
+mi_parse_print_values (const char *name)
+{
+   if (strcmp (name, "0") == 0
+       || strcmp (name, mi_no_values) == 0)
+     return PRINT_NO_VALUES;
+   else if (strcmp (name, "1") == 0
+	    || strcmp (name, mi_all_values) == 0)
+     return PRINT_ALL_VALUES;
+   else if (strcmp (name, "2") == 0
+	    || strcmp (name, mi_simple_values) == 0)
+     return PRINT_SIMPLE_VALUES;
+   else
+     error (_("Unknown value for PRINT_VALUES: must be: \
+0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
+	    mi_no_values, mi_all_values, mi_simple_values);
+}
diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
index 324ae5d..b20a389 100644
--- a/gdb/mi/mi-parse.h
+++ b/gdb/mi/mi-parse.h
@@ -66,4 +66,8 @@ extern struct mi_parse *mi_parse (const char *cmd, char **token);
 
 extern void mi_parse_free (struct mi_parse *cmd);
 
+/* Parse a string argument into a print_values value.  */
+
+enum print_values mi_parse_print_values (const char *name);
+
 #endif
-- 
1.7.7.6

Re: [PATCH] Remove parse_print_values

[Pedro Alves]

On 06/03/2013 04:18 AM, Yao Qi wrote:

> Patch below is what I committed.  Thanks for the review.

Thanks.

-- 
Pedro Alves