[PATCH] Make {get,set}_inferior_io_terminal inferior methods

Pedro Alves pedro@palves.net
Sat Jun 27 14:02:09 GMT 2020


On 6/25/20 10:22 PM, Simon Marchi wrote:
> On 2020-06-25 5:13 p.m., Pedro Alves wrote:
>> @@ -411,6 +406,13 @@ class inferior : public refcounted_object
>>    inline safe_inf_threads_range threads_safe ()
>>    { return safe_inf_threads_range (this->thread_list); }
>>  
>> +  /* Set/get file name for default use for standard in/out in the
>> +     inferior.  On Unix systems, we try to make TERMINAL_NAME the
>> +     inferior's controlling terminal.  Ownership of TERMINAL_NAME is
>> +     not transferred.  */
> 
> You could maybe mention that `nullptr` means to use the same terminal
> as GDB.

Indeed.

>> +private:
>>    /* The name of terminal device to use for I/O.  */
>> -  gdb::unique_xmalloc_ptr<char> terminal;
>> +  gdb::unique_xmalloc_ptr<char> m_terminal;
> 
> Is there a reason not to just move it in the existing "private" section at the bottom?  I don't
> think there's an efficiency / packing issue here.

Not really.

> 
> Otherwise LGTM.  I like that it pushes the reference to "current_inferior" to the callers.

Alright, I've pushed this then.  Thanks for the review.

>From 1e806939fa12da5989c937cb71e2f6b5546df1bb Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Sat, 27 Jun 2020 14:56:05 +0100
Subject: [PATCH] Make {get,set}_inferior_io_terminal inferior methods

This converts the get_inferior_io_terminal and
set_inferior_io_terminal free functions to inferior methods.

Since the related commands are called "tty", "{set,show}
inferior-tty", and MI's "-inferior-tty-{set,show}", to make the
connection between the commands and the code more obvious, the methods
are named set_tty/tty instead of set_io_terminal/io_terminal.

gdb/ChangeLog:

	* fork-child.c (prefork_hook): Adjust.
	* infcmd.c (set_inferior_io_terminal, get_inferior_io_terminal):
	Delete.
	(set_inferior_tty_command, show_inferior_tty_command): Adjust.
	* inferior.c (inferior::set_tty, inferior::tty): New methods.
	* inferior.h (set_inferior_io_terminal, get_inferior_io_terminal):
	Remove declarations.
	(struct inferior) <set_tty, tty>: New methods.
	(struct inferior) <terminal>: Rename to ...
	(struct inferior) <m_terminal>: ... this and make private.
	* main.c (captured_main_1): Adjust.
	* mi/mi-cmd-env.c (mi_cmd_inferior_tty_set): Adjust.
	(mi_cmd_inferior_tty_show): Adjust.
	* nto-procfs.c (nto_procfs_target::create_inferior): Adjust.
	* windows-nat.c (windows_nat_target::create_inferior): Adjust.
---
 gdb/fork-child.c    |  4 +---
 gdb/infcmd.c        | 29 +++++------------------------
 gdb/inferior.c      | 15 +++++++++++++++
 gdb/inferior.h      | 19 +++++++++++--------
 gdb/main.c          |  2 +-
 gdb/mi/mi-cmd-env.c |  9 ++++-----
 gdb/nto-procfs.c    | 10 +++++-----
 gdb/windows-nat.c   | 16 ++++++++--------
 8 files changed, 50 insertions(+), 54 deletions(-)

diff --git a/gdb/fork-child.c b/gdb/fork-child.c
index 90a01b2b16f..bcf077d0eba 100644
--- a/gdb/fork-child.c
+++ b/gdb/fork-child.c
@@ -61,8 +61,6 @@ static struct ui *saved_ui = NULL;
 void
 prefork_hook (const char *args)
 {
-  const char *inferior_io_terminal = get_inferior_io_terminal ();
-
   gdb_assert (saved_ui == NULL);
   /* Retain a copy of our UI, since the child will replace this value
      and if we're vforked, we have to restore it.  */
@@ -70,7 +68,7 @@ prefork_hook (const char *args)
 
   /* Tell the terminal handling subsystem what tty we plan to run on;
      it will just record the information for later.  */
-  new_tty_prefork (inferior_io_terminal);
+  new_tty_prefork (current_inferior ()->tty ());
 }
 
 /* See nat/fork-inferior.h.  */
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 17f7b9abeac..64eea883cb6 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -100,25 +100,6 @@ enum stop_stack_kind stop_stack_dummy;
 int stopped_by_random_signal;
 
 

-/* Accessor routines.  */
-
-/* Set the io terminal for the current inferior.  Ownership of
-   TERMINAL_NAME is not transferred.  */
-
-void 
-set_inferior_io_terminal (const char *terminal_name)
-{
-  if (terminal_name != NULL && *terminal_name != '\0')
-    current_inferior ()->terminal = make_unique_xstrdup (terminal_name);
-  else
-    current_inferior ()->terminal = NULL;
-}
-
-const char *
-get_inferior_io_terminal (void)
-{
-  return current_inferior ()->terminal.get ();
-}
 
 static void
 set_inferior_tty_command (const char *args, int from_tty,
@@ -126,7 +107,7 @@ set_inferior_tty_command (const char *args, int from_tty,
 {
   /* CLI has assigned the user-provided value to inferior_io_terminal_scratch.
      Now route it to current inferior.  */
-  set_inferior_io_terminal (inferior_io_terminal_scratch);
+  current_inferior ()->set_tty (inferior_io_terminal_scratch);
 }
 
 static void
@@ -135,13 +116,13 @@ show_inferior_tty_command (struct ui_file *file, int from_tty,
 {
   /* Note that we ignore the passed-in value in favor of computing it
      directly.  */
-  const char *inferior_io_terminal = get_inferior_io_terminal ();
+  const char *inferior_tty = current_inferior ()->tty ();
 
-  if (inferior_io_terminal == NULL)
-    inferior_io_terminal = "";
+  if (inferior_tty == nullptr)
+    inferior_tty = "";
   fprintf_filtered (gdb_stdout,
 		    _("Terminal for future runs of program being debugged "
-		      "is \"%s\".\n"), inferior_io_terminal);
+		      "is \"%s\".\n"), inferior_tty);
 }
 
 const char *
diff --git a/gdb/inferior.c b/gdb/inferior.c
index d3bece029dd..f775938721d 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -93,6 +93,21 @@ inferior::inferior (int pid_)
   m_target_stack.push (get_dummy_target ());
 }
 
+void
+inferior::set_tty (const char *terminal_name)
+{
+  if (terminal_name != nullptr && *terminal_name != '\0')
+    m_terminal = make_unique_xstrdup (terminal_name);
+  else
+    m_terminal = NULL;
+}
+
+const char *
+inferior::tty ()
+{
+  return m_terminal.get ();
+}
+
 struct inferior *
 add_inferior_silent (int pid)
 {
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 5002b0b8b3d..572c5f3ae72 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -118,11 +118,6 @@ extern void set_sigint_trap (void);
 
 extern void clear_sigint_trap (void);
 
-/* Set/get file name for default use for standard in/out in the inferior.  */
-
-extern void set_inferior_io_terminal (const char *terminal_name);
-extern const char *get_inferior_io_terminal (void);
-
 /* Collected pid, tid, etc. of the debugged inferior.  When there's
    no inferior, inferior_ptid.pid () will be 0.  */
 
@@ -411,6 +406,14 @@ class inferior : public refcounted_object
   inline safe_inf_threads_range threads_safe ()
   { return safe_inf_threads_range (this->thread_list); }
 
+  /* Set/get file name for default use for standard in/out in the
+     inferior.  On Unix systems, we try to make TERMINAL_NAME the
+     inferior's controlling terminal.  If TERMINAL_NAME is nullptr or
+     the empty string, then the inferior inherits GDB's terminal (or
+     GDBserver's if spawning a remote process).  */
+  void set_tty (const char *terminal_name);
+  const char *tty ();
+
   /* Convenient handle (GDB inferior id).  Unique across all
      inferiors.  */
   int num = 0;
@@ -456,9 +459,6 @@ class inferior : public refcounted_object
      this inferior.  */
   gdb::unique_xmalloc_ptr<char> cwd;
 
-  /* The name of terminal device to use for I/O.  */
-  gdb::unique_xmalloc_ptr<char> terminal;
-
   /* The terminal state as set by the last target_terminal::terminal_*
      call.  */
   target_terminal_state terminal_state = target_terminal_state::is_ours;
@@ -541,6 +541,9 @@ class inferior : public refcounted_object
 private:
   /* The inferior's target stack.  */
   target_stack m_target_stack;
+
+  /* The name of terminal device to use for I/O.  */
+  gdb::unique_xmalloc_ptr<char> m_terminal;
 };
 
 /* Keep a registry of per-inferior data-pointers required by other GDB
diff --git a/gdb/main.c b/gdb/main.c
index 3649e4a2201..19bbb923889 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -1171,7 +1171,7 @@ captured_main_1 (struct captured_main_args *context)
     }
 
   if (ttyarg != NULL)
-    set_inferior_io_terminal (ttyarg);
+    current_inferior ()->set_tty (ttyarg);
 
   /* Error messages should no longer be distinguished with extra output.  */
   warning_pre_print = _("warning: ");
diff --git a/gdb/mi/mi-cmd-env.c b/gdb/mi/mi-cmd-env.c
index 88158cee56f..52ba0065bba 100644
--- a/gdb/mi/mi-cmd-env.c
+++ b/gdb/mi/mi-cmd-env.c
@@ -244,7 +244,7 @@ mi_cmd_env_dir (const char *command, char **argv, int argc)
 void
 mi_cmd_inferior_tty_set (const char *command, char **argv, int argc)
 {
-  set_inferior_io_terminal (argv[0]);
+  current_inferior ()->set_tty (argv[0]);
 }
 
 /* Print the inferior terminal device name.  */
@@ -252,13 +252,12 @@ mi_cmd_inferior_tty_set (const char *command, char **argv, int argc)
 void
 mi_cmd_inferior_tty_show (const char *command, char **argv, int argc)
 {
-  const char *inferior_io_terminal = get_inferior_io_terminal ();
-  
   if ( !mi_valid_noargs ("-inferior-tty-show", argc, argv))
     error (_("-inferior-tty-show: Usage: No args"));
 
-  if (inferior_io_terminal)
-    current_uiout->field_string ("inferior_tty_terminal", inferior_io_terminal);
+  const char *inferior_tty = current_inferior ()->tty ();
+  if (inferior_tty != NULL)
+    current_uiout->field_string ("inferior_tty_terminal", inferior_tty);
 }
 
 void _initialize_mi_cmd_env ();
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index f649b6cf58b..91d2cc5914d 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -1208,7 +1208,6 @@ nto_procfs_target::create_inferior (const char *exec_file,
   const char *in = "", *out = "", *err = "";
   int fd, fds[3];
   sigset_t set;
-  const char *inferior_io_terminal = get_inferior_io_terminal ();
   struct inferior *inf;
 
   argv = xmalloc ((allargs.size () / (unsigned) 2 + 2) *
@@ -1233,14 +1232,15 @@ nto_procfs_target::create_inferior (const char *exec_file,
 
   /* If the user specified I/O via gdb's --tty= arg, use it, but only
      if the i/o is not also being specified via redirection.  */
-  if (inferior_io_terminal)
+  const char *inferior_tty = current_inferior ()->tty ();
+  if (inferior_tty != nullptr)
     {
       if (!in[0])
-	in = inferior_io_terminal;
+	in = inferior_tty;
       if (!out[0])
-	out = inferior_io_terminal;
+	out = inferior_tty;
       if (!err[0])
-	err = inferior_io_terminal;
+	err = inferior_tty;
     }
 
   if (in[0])
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 68df87d1bf2..188a920cbb0 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -2730,7 +2730,7 @@ windows_nat_target::create_inferior (const char *exec_file,
   PROCESS_INFORMATION pi;
   BOOL ret;
   DWORD flags = 0;
-  const char *inferior_io_terminal = get_inferior_io_terminal ();
+  const char *inferior_tty = current_inferior ()->tty ();
 
   if (!exec_file)
     error (_("No executable specified, use `target exec'."));
@@ -2829,14 +2829,14 @@ windows_nat_target::create_inferior (const char *exec_file,
       w32_env = NULL;
     }
 
-  if (!inferior_io_terminal)
+  if (inferior_tty == nullptr)
     tty = ostdin = ostdout = ostderr = -1;
   else
     {
-      tty = open (inferior_io_terminal, O_RDWR | O_NOCTTY);
+      tty = open (inferior_tty, O_RDWR | O_NOCTTY);
       if (tty < 0)
 	{
-	  print_sys_errmsg (inferior_io_terminal, errno);
+	  print_sys_errmsg (inferior_tty, errno);
 	  ostdin = ostdout = ostderr = -1;
 	}
       else
@@ -2901,19 +2901,19 @@ windows_nat_target::create_inferior (const char *exec_file,
       allargs_len = strlen (allargs_copy);
     }
   /* If not all the standard streams are redirected by the command
-     line, use inferior_io_terminal for those which aren't.  */
-  if (inferior_io_terminal
+     line, use INFERIOR_TTY for those which aren't.  */
+  if (inferior_tty != nullptr
       && !(fd_inp >= 0 && fd_out >= 0 && fd_err >= 0))
     {
       SECURITY_ATTRIBUTES sa;
       sa.nLength = sizeof(sa);
       sa.lpSecurityDescriptor = 0;
       sa.bInheritHandle = TRUE;
-      tty = CreateFileA (inferior_io_terminal, GENERIC_READ | GENERIC_WRITE,
+      tty = CreateFileA (inferior_tty, GENERIC_READ | GENERIC_WRITE,
 			 0, &sa, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, 0);
       if (tty == INVALID_HANDLE_VALUE)
 	warning (_("Warning: Failed to open TTY %s, error %#x."),
-		 inferior_io_terminal, (unsigned) GetLastError ());
+		 inferior_tty, (unsigned) GetLastError ());
     }
   if (redirected || tty != INVALID_HANDLE_VALUE)
     {

base-commit: cfc16775b7678e1ad8f9fce048652defd78e3787
-- 
2.14.5



More information about the Gdb-patches mailing list