* [obv] Handle var_string_noescape and var_optional_filename together. @ 2012-07-18 12:41 Yao Qi 2012-07-18 13:57 ` Jan Kratochvil 0 siblings, 1 reply; 6+ messages in thread From: Yao Qi @ 2012-07-18 12:41 UTC (permalink / raw) To: gdb-patches Hi, I notice that the code in case var_string_noescape and var_optional_filename are exactly the same, so this patch is to combine them together. I'll commit it in two days. gdb: 2012-07-18 Yao Qi <yao@codesourcery.com> * cli/cli-setshow.c (do_setshow_command): Handle case 'var_string_noescape' and 'var_optional_filename' together. --- gdb/cli/cli-setshow.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-) diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c index f46d15a..0f854e5 100644 --- a/gdb/cli/cli-setshow.c +++ b/gdb/cli/cli-setshow.c @@ -190,12 +190,6 @@ do_setshow_command (char *arg, int from_tty, struct cmd_list_element *c) } break; case var_string_noescape: - if (arg == NULL) - arg = ""; - if (*(char **) c->var != NULL) - xfree (*(char **) c->var); - *(char **) c->var = xstrdup (arg); - break; case var_optional_filename: if (arg == NULL) arg = ""; -- 1.7.7.6 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [obv] Handle var_string_noescape and var_optional_filename together. 2012-07-18 12:41 [obv] Handle var_string_noescape and var_optional_filename together Yao Qi @ 2012-07-18 13:57 ` Jan Kratochvil 2012-07-18 14:59 ` Yao Qi 0 siblings, 1 reply; 6+ messages in thread From: Jan Kratochvil @ 2012-07-18 13:57 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches On Wed, 18 Jul 2012 20:40:14 +0200, Yao Qi wrote: > I notice that the code in case var_string_noescape and var_optional_filename > are exactly the same, so this patch is to combine them together. I'll commit > it in two days. I do not agree, this will hide an existing bug. Correct it to fix var_optional_filename to be more like var_filename (clearing whitespaces + doing tilde_expand); except sure without that: if (arg == NULL) error_no_arg (_("filename to set it to.")); Thanks, Jan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [obv] Handle var_string_noescape and var_optional_filename together. 2012-07-18 13:57 ` Jan Kratochvil @ 2012-07-18 14:59 ` Yao Qi 2012-07-18 15:26 ` Jan Kratochvil 0 siblings, 1 reply; 6+ messages in thread From: Yao Qi @ 2012-07-18 14:59 UTC (permalink / raw) To: gdb-patches; +Cc: Jan Kratochvil On Wednesday, July 18, 2012 03:56:55 PM Jan Kratochvil wrote: > On Wed, 18 Jul 2012 20:40:14 +0200, Yao Qi wrote: > > I notice that the code in case var_string_noescape and > > var_optional_filename are exactly the same, so this patch is to combine > > them together. I'll commit it in two days. > > I do not agree, this will hide an existing bug. > Oh, I didn't realize that there may be a bug when handling var_optional_filename here. > Correct it to fix var_optional_filename to be more like var_filename > (clearing whitespaces + doing tilde_expand); except sure without that: > if (arg == NULL) > error_no_arg (_("filename to set it to.")); How about this one? -- Yao (齐尧) gdb: 2012-07-18 Yao Qi <yao@codesourcery.com> * cli/cli-setshow.c (do_setshow_command): Handle case 'var_filename' and case 'var_optional_filename' together. --- gdb/cli/cli-setshow.c | 14 +++++++------- 1 files changed, 7 insertions(+), 7 deletions(-) diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c index 521ac0e..e44a35e 100644 --- a/gdb/cli/cli-setshow.c +++ b/gdb/cli/cli-setshow.c @@ -183,15 +183,15 @@ do_setshow_command (char *arg, int from_tty, struct cmd_list_element *c) *(char **) c->var = xstrdup (arg); break; case var_optional_filename: - if (arg == NULL) - arg = ""; - if (*(char **) c->var != NULL) - xfree (*(char **) c->var); - *(char **) c->var = xstrdup (arg); - break; case var_filename: if (arg == NULL) - error_no_arg (_("filename to set it to.")); + { + if (c->var_type == var_filename) + error_no_arg (_("filename to set it to.")); + else + arg = ""; + } + if (*(char **) c->var != NULL) xfree (*(char **) c->var); { -- 1.7.7.6 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [obv] Handle var_string_noescape and var_optional_filename together. 2012-07-18 14:59 ` Yao Qi @ 2012-07-18 15:26 ` Jan Kratochvil 2012-07-19 7:54 ` [committed]:[obv] " Yao Qi 0 siblings, 1 reply; 6+ messages in thread From: Jan Kratochvil @ 2012-07-18 15:26 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches On Wed, 18 Jul 2012 22:59:15 +0200, Yao Qi wrote: > How about this one? Just it has a sort-of-regression: (gdb) set args ~ (gdb) show args Argument list to give program being debugged when it is started is "~". -> Argument list to give program being debugged when it is started is "/home/jkratoch". Therefore please modify also: add_setshow_optional_filename_cmd ("args", class_run, to rather use add_setshow_string_noescape_cmd together with: set_cmd_completer (set_result, filename_completer); It should not affect anything else I think. Pre-approved with that change. Thanks, Jan ^ permalink raw reply [flat|nested] 6+ messages in thread
* [committed]:[obv] Handle var_string_noescape and var_optional_filename together. 2012-07-18 15:26 ` Jan Kratochvil @ 2012-07-19 7:54 ` Yao Qi 2012-07-19 7:58 ` Jan Kratochvil 0 siblings, 1 reply; 6+ messages in thread From: Yao Qi @ 2012-07-19 7:54 UTC (permalink / raw) To: gdb-patches; +Cc: Jan Kratochvil On Wednesday, July 18, 2012 05:26:15 PM Jan Kratochvil wrote: > Just it has a sort-of-regression: > (gdb) set args ~ > (gdb) show args > Argument list to give program being debugged when it is started is "~". > -> > Argument list to give program being debugged when it is started is > "/home/jkratoch". > I don't see "set args ~" in testsuite, so I add it into gdb.base/setshow.exp. > Therefore please modify also: > add_setshow_optional_filename_cmd ("args", class_run, > > to rather use add_setshow_string_noescape_cmd together with: > set_cmd_completer (set_result, filename_completer); > Done as you suggested. Regression tested on x86_64 native gdb. Committed. Jan, thanks for the review. -- Yao (齐尧) gdb: 2012-07-20 Yao Qi <yao@codesourcery.com> Jan Kratochvil <jan.kratochvil@redhat.com> * cli/cli-setshow.c (do_setshow_command): Handle case 'var_filename' and case 'var_optional_filename' together. * infcmd.c (_initialize_infcmd): Call add_setshow_string_noescape_cmd instead of add_setshow_optional_filename_cmd for setshow command 'args'. Set completer for 'set args'. gdb/testsuite: 2012-07-20 Yao Qi <yao@codesourcery.com> * gdb.base/setshow.exp: Test 'set args ~'. --- gdb/cli/cli-setshow.c | 30 +++++++++++++++--------------- gdb/infcmd.c | 15 ++++++++++----- gdb/testsuite/gdb.base/setshow.exp | 4 ++++ 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c index 521ac0e..dccf425 100644 --- a/gdb/cli/cli-setshow.c +++ b/gdb/cli/cli-setshow.c @@ -182,27 +182,27 @@ do_setshow_command (char *arg, int from_tty, struct cmd_list_element *c) xfree (*(char **) c->var); *(char **) c->var = xstrdup (arg); break; - case var_optional_filename: - if (arg == NULL) - arg = ""; - if (*(char **) c->var != NULL) - xfree (*(char **) c->var); - *(char **) c->var = xstrdup (arg); - break; case var_filename: if (arg == NULL) error_no_arg (_("filename to set it to.")); + /* FALLTHROUGH */ + case var_optional_filename: if (*(char **) c->var != NULL) xfree (*(char **) c->var); - { - /* Clear trailing whitespace of filename. */ - char *ptr = arg + strlen (arg) - 1; - while (ptr >= arg && (*ptr == ' ' || *ptr == '\t')) - ptr--; - *(ptr + 1) = '\0'; - } - *(char **) c->var = tilde_expand (arg); + if (arg != NULL) + { + /* Clear trailing whitespace of filename. */ + char *ptr = arg + strlen (arg) - 1; + + while (ptr >= arg && (*ptr == ' ' || *ptr == '\t')) + ptr--; + *(ptr + 1) = '\0'; + + *(char **) c->var = tilde_expand (arg); + } + else + *(char **) c->var = xstrdup (""); break; case var_boolean: *(int *) c->var = parse_binary_operation (arg); diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 475ac90..635e577 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -2918,6 +2918,7 @@ _initialize_infcmd (void) { static struct cmd_list_element *info_proc_cmdlist; struct cmd_list_element *c = NULL; + char *cmd_name; /* Add the filename of the terminal connected to inferior I/O. */ add_setshow_filename_cmd ("inferior-tty", class_run, @@ -2930,14 +2931,18 @@ Usage: set inferior-tty /dev/pts/1"), &setlist, &showlist); add_com_alias ("tty", "set inferior-tty", class_alias, 0); - add_setshow_optional_filename_cmd ("args", class_run, - &inferior_args_scratch, _("\ + cmd_name = "args"; + add_setshow_string_noescape_cmd (cmd_name, class_run, + &inferior_args_scratch, _("\ Set argument list to give program being debugged when it is started."), _("\ Show argument list to give program being debugged when it is started."), _("\ Follow this command with any number of args, to be passed to the program."), - set_args_command, - show_args_command, - &setlist, &showlist); + set_args_command, + show_args_command, + &setlist, &showlist); + c = lookup_cmd (&cmd_name, setlist, "", -1, 1); + gdb_assert (c != NULL); + set_cmd_completer (c, filename_completer); c = add_cmd ("environment", no_class, environment_info, _("\ The environment to give the program, or one variable's value.\n\ diff --git a/gdb/testsuite/gdb.base/setshow.exp b/gdb/testsuite/gdb.base/setshow.exp index b218314..9af5c30 100644 --- a/gdb/testsuite/gdb.base/setshow.exp +++ b/gdb/testsuite/gdb.base/setshow.exp @@ -87,6 +87,10 @@ gdb_test_no_output "set annotate 0" "set annotate 0" gdb_test "show annotate" "Annotation_level is 0..*" "show annotate (0)" #test annotation_level 0 gdb_test "info line 1" "Line 1 of .* is at address .* but contains no code.*" "annotation_level 0" + +gdb_test_no_output "set args ~" +gdb_test "show args" "Argument list to give program being debugged when it is started is \"~\"..*" \ + "show args ~" #test set args gdb_test_no_output "set args foo bar blup baz bubble" "set args" #test show args -- 1.7.7.6 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [committed]:[obv] Handle var_string_noescape and var_optional_filename together. 2012-07-19 7:54 ` [committed]:[obv] " Yao Qi @ 2012-07-19 7:58 ` Jan Kratochvil 0 siblings, 0 replies; 6+ messages in thread From: Jan Kratochvil @ 2012-07-19 7:58 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches On Thu, 19 Jul 2012 09:53:21 +0200, Yao Qi wrote: > Done as you suggested. Regression tested on x86_64 native gdb. Committed. > Jan, thanks for the review. Yes, it looks OK to me. Thanks, Jan ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-07-19 7:58 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-07-18 12:41 [obv] Handle var_string_noescape and var_optional_filename together Yao Qi 2012-07-18 13:57 ` Jan Kratochvil 2012-07-18 14:59 ` Yao Qi 2012-07-18 15:26 ` Jan Kratochvil 2012-07-19 7:54 ` [committed]:[obv] " Yao Qi 2012-07-19 7:58 ` Jan Kratochvil
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox