* [PATCH] Remove parse_print_values @ 2013-05-31 14:16 Yao Qi 2013-05-31 14:40 ` Pedro Alves 0 siblings, 1 reply; 4+ messages in thread From: Yao Qi @ 2013-05-31 14:16 UTC (permalink / raw) To: gdb-patches 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Remove parse_print_values 2013-05-31 14:16 [PATCH] Remove parse_print_values Yao Qi @ 2013-05-31 14:40 ` Pedro Alves 2013-06-03 3:18 ` Yao Qi 0 siblings, 1 reply; 4+ messages in thread From: Pedro Alves @ 2013-05-31 14:40 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Remove parse_print_values 2013-05-31 14:40 ` Pedro Alves @ 2013-06-03 3:18 ` Yao Qi 2013-06-03 11:36 ` Pedro Alves 0 siblings, 1 reply; 4+ messages in thread From: Yao Qi @ 2013-06-03 3:18 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Remove parse_print_values 2013-06-03 3:18 ` Yao Qi @ 2013-06-03 11:36 ` Pedro Alves 0 siblings, 0 replies; 4+ messages in thread From: Pedro Alves @ 2013-06-03 11:36 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches On 06/03/2013 04:18 AM, Yao Qi wrote: > Patch below is what I committed. Thanks for the review. Thanks. -- Pedro Alves ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-06-03 11:36 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-05-31 14:16 [PATCH] Remove parse_print_values Yao Qi 2013-05-31 14:40 ` Pedro Alves 2013-06-03 3:18 ` Yao Qi 2013-06-03 11:36 ` Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox