[PATCH] Allow resetting an empty inferior-tty

[PATCH] Allow resetting an empty inferior-tty

[Simon Marchi]

This patch allows the user to set the inferior-tty to "empty", in order
to come back to the default behaviour of using the same tty as gdb is
using.

This is already supported in MI (and tested in gdb.mi/mi-basics.exp).

I added a new test, set-inferior-tty.exp, where I test only the setting
and unsetting of the parameter.  It would be nice to actually test that
the inferior output properly goes to the separate tty, but that will be
for another day.

gdb/ChangeLog:

	* infcmd.c (set_inferior_io_terminal): Set inferior terminal to
	NULL if terminal_name is an empty string.
	(_initialize_infcmd): Make the argument of "set inferior-tty"
	optional.

gdb/doc/ChangeLog:

	* gdb.texinfo (Your Program’s Input and Output): Mention
	possibility to unset inferior-tty.

gdb/testsuite/ChangeLog:

	* gdb.base/set-inferior-tty.exp: New file.
	* gdb.base/set-inferior-tty.c: New file.
---
 gdb/doc/gdb.texinfo                         |  6 +++--
 gdb/infcmd.c                                | 16 +++++++-----
 gdb/testsuite/gdb.base/set-inferior-tty.c   | 24 +++++++++++++++++
 gdb/testsuite/gdb.base/set-inferior-tty.exp | 40 +++++++++++++++++++++++++++++
 4 files changed, 78 insertions(+), 8 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/set-inferior-tty.c
 create mode 100644 gdb/testsuite/gdb.base/set-inferior-tty.exp

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f5dde61..f00729a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2499,9 +2499,11 @@ display the name of the terminal that will be used for future runs of your
 program.
 
 @table @code
-@item set inferior-tty /dev/ttyb
+@item set inferior-tty @var{tty}
 @kindex set inferior-tty
-Set the tty for the program being debugged to /dev/ttyb.
+Set the tty for the program being debugged to @var{tty}.  Omitting @var{tty}
+restores the default behavior, which is to use the same terminal as
+@value{GDBN}.
 
 @item show inferior-tty
 @kindex show inferior-tty
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 58ba1cb..97a1e35 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -151,7 +151,11 @@ void
 set_inferior_io_terminal (const char *terminal_name)
 {
   xfree (current_inferior ()->terminal);
-  current_inferior ()->terminal = terminal_name ? xstrdup (terminal_name) : 0;
+
+  if (terminal_name != NULL && strlen (terminal_name) > 0)
+    current_inferior ()->terminal = xstrdup (terminal_name);
+  else
+    current_inferior ()->terminal = NULL;
 }
 
 const char *
@@ -3224,14 +3228,14 @@ _initialize_infcmd (void)
   const char *cmd_name;
 
   /* Add the filename of the terminal connected to inferior I/O.  */
-  add_setshow_filename_cmd ("inferior-tty", class_run,
-			    &inferior_io_terminal_scratch, _("\
+  add_setshow_optional_filename_cmd ("inferior-tty", class_run,
+				     &inferior_io_terminal_scratch, _("\
 Set terminal for future runs of program being debugged."), _("\
 Show terminal for future runs of program being debugged."), _("\
 Usage: set inferior-tty /dev/pts/1"),
-			    set_inferior_tty_command,
-			    show_inferior_tty_command,
-			    &setlist, &showlist);
+				     set_inferior_tty_command,
+				     show_inferior_tty_command,
+				     &setlist, &showlist);
   add_com_alias ("tty", "set inferior-tty", class_alias, 0);
 
   cmd_name = "args";
diff --git a/gdb/testsuite/gdb.base/set-inferior-tty.c b/gdb/testsuite/gdb.base/set-inferior-tty.c
new file mode 100644
index 0000000..863fa99
--- /dev/null
+++ b/gdb/testsuite/gdb.base/set-inferior-tty.c
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/set-inferior-tty.exp b/gdb/testsuite/gdb.base/set-inferior-tty.exp
new file mode 100644
index 0000000..1a5f49c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/set-inferior-tty.exp
@@ -0,0 +1,40 @@
+# Copyright 2016 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+set compile_options "debug"
+if {[build_executable $testfile.exp $testfile ${srcfile} ${compile_options}] == -1} {
+    untested "failed to compile $testfile"
+    return -1
+}
+
+proc test_set_inferior_tty { } {
+    global binfile
+
+    clean_restart ${binfile}
+
+    gdb_test_no_output "set inferior-tty hello" "set inferior-tty to hello"
+    gdb_test "show inferior-tty" \
+	     "Terminal for future runs of program being debugged is \"hello\"." \
+	     "show inferior-tty shows hello"
+
+    gdb_test_no_output "set inferior-tty" "set inferior-tty to empty"
+    gdb_test "show inferior-tty" \
+	     "Terminal for future runs of program being debugged is \"\"." \
+	     "show inferior-tty shows empty"
+}
+
+test_set_inferior_tty
-- 
2.9.3

Re: [PATCH] Allow resetting an empty inferior-tty

[Eli Zaretskii]

> From: Simon Marchi <simon.marchi@ericsson.com>
> CC: Simon Marchi <simon.marchi@ericsson.com>
> Date: Wed, 24 Aug 2016 10:45:48 -0400
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index f5dde61..f00729a 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -2499,9 +2499,11 @@ display the name of the terminal that will be used for future runs of your
>  program.
>  
>  @table @code
> -@item set inferior-tty /dev/ttyb
> +@item set inferior-tty @var{tty}
>  @kindex set inferior-tty
> -Set the tty for the program being debugged to /dev/ttyb.
> +Set the tty for the program being debugged to @var{tty}.  Omitting @var{tty}
> +restores the default behavior, which is to use the same terminal as
> +@value{GDBN}.

Thanks.  If the argument is optional, please show it as such, as we do
elsewhere in the manual.

OK with this nit fixed.

P.S.  Btw, it is strange that your log entry says

  gdb/doc/ChangeLog:

	  * gdb.texinfo (Your Program’s Input and Output): Mention
	  possibility to unset inferior-tty.

whereas the node's name is "Input/Output", and the section's name is
"Your Program's Input and Output", without any non-ASCII characters.
What happened there?

Re: [PATCH] Allow resetting an empty inferior-tty

[Simon Marchi]

On 16-08-24 11:05 AM, Eli Zaretskii wrote:
> Thanks.  If the argument is optional, please show it as such, as we do
> elsewhere in the manual.

Does that mean with square brackets?

  @item set inferior-tty [ @var{tty} ]

I see some commands with optional arguments that use them, but many others
(break, frame, list, etc.) do not, so I wasn't sure.

> OK with this nit fixed.
> 
> P.S.  Btw, it is strange that your log entry says
> 
>   gdb/doc/ChangeLog:
> 
> 	  * gdb.texinfo (Your Program’s Input and Output): Mention
> 	  possibility to unset inferior-tty.
> 
> whereas the node's name is "Input/Output", and the section's name is
> "Your Program's Input and Output", without any non-ASCII characters.
> What happened there?

I copied the title from the generated html version, and it seems like the
single quote is being changed to an actual apostrophe.  I'll change it to
a single quote, as it is in the source file.  Or is it better if I use the
node name instead (Input/Output)?

Thanks for the quick review.

Simon

Re: [PATCH] Allow resetting an empty inferior-tty

[Pedro Alves]

On 08/24/2016 03:45 PM, Simon Marchi wrote:
> This patch allows the user to set the inferior-tty to "empty", in order
> to come back to the default behaviour of using the same tty as gdb is
> using.

LGTM.  A couple nits below.

> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 58ba1cb..97a1e35 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -151,7 +151,11 @@ void
>  set_inferior_io_terminal (const char *terminal_name)
>  {
>    xfree (current_inferior ()->terminal);
> -  current_inferior ()->terminal = terminal_name ? xstrdup (terminal_name) : 0;
> +
> +  if (terminal_name != NULL && strlen (terminal_name) > 0)

Use th idiomatic empty-string check: *terminal_name != '\0'

>    /* Add the filename of the terminal connected to inferior I/O.  */
> -  add_setshow_filename_cmd ("inferior-tty", class_run,
> -			    &inferior_io_terminal_scratch, _("\
> +  add_setshow_optional_filename_cmd ("inferior-tty", class_run,
> +				     &inferior_io_terminal_scratch, _("\
>  Set terminal for future runs of program being debugged."), _("\
>  Show terminal for future runs of program being debugged."), _("\
>  Usage: set inferior-tty /dev/pts/1"),

Should we update the online help too?

Thanks,
Pedro Alves

Re: [PATCH] Allow resetting an empty inferior-tty

[Simon Marchi]

On 16-08-24 11:33 AM, Pedro Alves wrote:
>> +  if (terminal_name != NULL && strlen (terminal_name) > 0)
> 
> Use th idiomatic empty-string check: *terminal_name != '\0'

Done.

>>    /* Add the filename of the terminal connected to inferior I/O.  */
>> -  add_setshow_filename_cmd ("inferior-tty", class_run,
>> -			    &inferior_io_terminal_scratch, _("\
>> +  add_setshow_optional_filename_cmd ("inferior-tty", class_run,
>> +				     &inferior_io_terminal_scratch, _("\
>>  Set terminal for future runs of program being debugged."), _("\
>>  Show terminal for future runs of program being debugged."), _("\
>>  Usage: set inferior-tty /dev/pts/1"),
> 
> Should we update the online help too?

The online help is generated from the doc, which I updated, so it should
be ok, no?  Unless you are talking about something else?

Re: [PATCH] Allow resetting an empty inferior-tty

[Pedro Alves]

On 08/24/2016 04:36 PM, Simon Marchi wrote:
> On 16-08-24 11:33 AM, Pedro Alves wrote:

>>>    /* Add the filename of the terminal connected to inferior I/O.  */
>>> -  add_setshow_filename_cmd ("inferior-tty", class_run,
>>> -			    &inferior_io_terminal_scratch, _("\
>>> +  add_setshow_optional_filename_cmd ("inferior-tty", class_run,
>>> +				     &inferior_io_terminal_scratch, _("\
>>>  Set terminal for future runs of program being debugged."), _("\
>>>  Show terminal for future runs of program being debugged."), _("\
>>>  Usage: set inferior-tty /dev/pts/1"),
>>
>> Should we update the online help too?
> 
> The online help is generated from the doc, which I updated, so it should
> be ok, no?  Unless you are talking about something else?
> 

Something else.  :-)

I meant "(gdb) help set inferior-tty", the bit quoted above.

Thanks,
Pedro Alves

Re: [PATCH] Allow resetting an empty inferior-tty

[Simon Marchi]

On 16-08-24 11:41 AM, Pedro Alves wrote:
> Something else.  :-)
> 
> I meant "(gdb) help set inferior-tty", the bit quoted above.

Ah, that kind of online :P

What about:

@@ -3224,14 +3228,16 @@ _initialize_infcmd (void)
   const char *cmd_name;

   /* Add the filename of the terminal connected to inferior I/O.  */
-  add_setshow_filename_cmd ("inferior-tty", class_run,
-			    &inferior_io_terminal_scratch, _("\
+  add_setshow_optional_filename_cmd ("inferior-tty", class_run,
+				     &inferior_io_terminal_scratch, _("\
 Set terminal for future runs of program being debugged."), _("\
 Show terminal for future runs of program being debugged."), _("\
-Usage: set inferior-tty /dev/pts/1"),
-			    set_inferior_tty_command,
-			    show_inferior_tty_command,
-			    &setlist, &showlist);
+Usage: set inferior-tty [TTY]\n\n\
+If TTY is omitted, the default behavior of using the same terminal as GDB\n\
+is restored."),
+				     set_inferior_tty_command,
+				     show_inferior_tty_command,
+				     &setlist, &showlist);
   add_com_alias ("tty", "set inferior-tty", class_alias, 0);

   cmd_name = "args";


For this result:

(gdb) help set inferior-tty
Set terminal for future runs of program being debugged.
Usage: set inferior-tty [TTY]

If TTY is omitted, the default behavior of using the same terminal as GDB
is restored.
(gdb)

Re: [PATCH] Allow resetting an empty inferior-tty

[Eli Zaretskii]

> CC: <gdb-patches@sourceware.org>
> From: Simon Marchi <simon.marchi@ericsson.com>
> Date: Wed, 24 Aug 2016 11:20:51 -0400
> 
> On 16-08-24 11:05 AM, Eli Zaretskii wrote:
> > Thanks.  If the argument is optional, please show it as such, as we do
> > elsewhere in the manual.
> 
> Does that mean with square brackets?
> 
>   @item set inferior-tty [ @var{tty} ]

Yes.

> > P.S.  Btw, it is strange that your log entry says
> > 
> >   gdb/doc/ChangeLog:
> > 
> > 	  * gdb.texinfo (Your Program’s Input and Output): Mention
> > 	  possibility to unset inferior-tty.
> > 
> > whereas the node's name is "Input/Output", and the section's name is
> > "Your Program's Input and Output", without any non-ASCII characters.
> > What happened there?
> 
> I copied the title from the generated html version, and it seems like the
> single quote is being changed to an actual apostrophe.

Ah, okay, now it's clear.

> I'll change it to a single quote, as it is in the source file.  Or
> is it better if I use the node name instead (Input/Output)?

Please use the node name, as we always do.

Re: [PATCH] Allow resetting an empty inferior-tty

[Pedro Alves]

On 08/24/2016 04:50 PM, Simon Marchi wrote:

> For this result:
> 
> (gdb) help set inferior-tty
> Set terminal for future runs of program being debugged.
> Usage: set inferior-tty [TTY]
> 
> If TTY is omitted, the default behavior of using the same terminal as GDB
> is restored.
> (gdb)
> 

LGTM.

Thanks,
Pedro Alves

Re: [PATCH] Allow resetting an empty inferior-tty

[Simon Marchi]

On 16-08-24 12:20 PM, Pedro Alves wrote:
> LGTM.
> 
> Thanks,
> Pedro Alves

Thanks, pushed!

Re: [PATCH] Allow resetting an empty inferior-tty

[Simon Marchi]

On 16-08-24 01:10 PM, Simon Marchi wrote:
> On 16-08-24 12:20 PM, Pedro Alves wrote:
>> LGTM.
>>
>> Thanks,
>> Pedro Alves
> 
> Thanks, pushed!

As per Pedro's suggestion, I also pushed this to the 7.12 branch.

[testsuite patch+7.12] gdb.base/default.exp regression [Re: [PATCH] Allow resetting an empty inferior-tty]

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 193 bytes --]

On Wed, 24 Aug 2016 19:18:15 +0200, Simon Marchi wrote:
> As per Pedro's suggestion, I also pushed this to the 7.12 branch.

tty^M
(gdb) FAIL: gdb.base/default.exp: tty

OK for check-in?


Jan

[-- Attachment #2: 1 --]
[-- Type: text/plain, Size: 755 bytes --]

gdb/testsuite/ChangeLog
2016-08-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/default.exp (tty): Remove.

diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index 612411b..38c3d4a 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -799,8 +799,6 @@ gdb_test "thread apply" "Please specify a thread ID list" "thread apply"
 gdb_test "thread find" "Command requires an argument." "thread find"
 #test thread name
 gdb_test "thread name" "No thread selected" "thread name"
-#test tty
-gdb_test "tty" "Argument required .filename to set it to\..*" "tty"
 #test until "u" abbreviation
 gdb_test "u" "The program is not being run." "until \"u\" abbreviation"
 #test until

Re: [testsuite patch+7.12] gdb.base/default.exp regression [Re: [PATCH] Allow resetting an empty inferior-tty]

[Simon Marchi]

On 2016-08-29 06:55, Jan Kratochvil wrote:
> On Wed, 24 Aug 2016 19:18:15 +0200, Simon Marchi wrote:
>> As per Pedro's suggestion, I also pushed this to the 7.12 branch.
> 
> tty^M
> (gdb) FAIL: gdb.base/default.exp: tty
> 
> OK for check-in?
> 
> 
> Jan

Derp.  Thanks for this!

Re: [testsuite patch+7.12] gdb.base/default.exp regression [Re: [PATCH] Allow resetting an empty inferior-tty]

[Pedro Alves]

On 08/29/2016 11:55 AM, Jan Kratochvil wrote:
> On Wed, 24 Aug 2016 19:18:15 +0200, Simon Marchi wrote:
>> As per Pedro's suggestion, I also pushed this to the 7.12 branch.
> 
> tty^M
> (gdb) FAIL: gdb.base/default.exp: tty
> 
> OK for check-in?

OK.

Thanks,
Pedro Alves

[commit] [testsuite patch+7.12] gdb.base/default.exp regression [Re: [PATCH] Allow resetting an empty inferior-tty]

[Jan Kratochvil]

On Mon, 29 Aug 2016 15:01:30 +0200, Pedro Alves wrote:
> On 08/29/2016 11:55 AM, Jan Kratochvil wrote:
> > tty^M
> > (gdb) FAIL: gdb.base/default.exp: tty
> > 
> > OK for check-in?
> 
> OK.

Checked in:
	7fac69100a7c1fb52b2e044294a858272bad4e46
and for 7.12:
	b82dedc7696daab1b7a4a28530f3eb94c6b4e62d


Jan