* [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