public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 1/4] Make common code handle target_terminal_* idempotency
Date: Thu, 09 Oct 2014 18:00:00 -0000	[thread overview]
Message-ID: <1412877629-12052-2-git-send-email-palves@redhat.com> (raw)
In-Reply-To: <1412877629-12052-1-git-send-email-palves@redhat.com>

I found a place that should be giving back the terminal to the target,
but only if the target was already owning it.  So I need to add a
getter for who owns the terminal.

The trouble is that several places/target have their own globals to
track this state:

 - inflow.c:terminal_is_ours
 - remote.c:remote_async_terminal_ours_p
 - linux-nat.c:async_terminal_is_ours
 - go32-nat.c:terminal_is_ours

While one might think of adding a new target_ops method to query this,
conceptually, this state isn't really part of a particular target_ops.
Considering multi-target, the core shouldn't have to ask all targets
to know whether it's GDB that owns the terminal.  There's only one GDB
(or rather, only one top level interpreter).

So what this comment does is add a new global that is tracked by the
core instead.  A subsequent pass may later remove the other globals.

Tested on x86_64 Fedora 20, native and gdbserver.

gdb/
2014-10-09  Pedro Alves  <palves@redhat.com>

	* target.c (enum terminal_state): New enum.
	(terminal_state): New global.
	(target_terminal_init): New function.
	(target_terminal_inferior): Skip if inferior already owns the
	terminal.
	(target_terminal_ours, target_terminal_ours_for_output): New
	functions.
	* target.h (target_terminal_init): Convert to function prototype.
	(target_terminal_ours_for_output): Convert to function prototype
	and tweak comment.
	(target_terminal_ours): Convert to function prototype and tweak
	comment.
	* windows-nat.c (do_initial_windows_stuff): Call
	target_terminal_init instead of child_terminal_init_with_pgrp.
---
 gdb/target.c      | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/target.h      | 20 +++++++-------------
 gdb/windows-nat.c |  2 +-
 3 files changed, 64 insertions(+), 14 deletions(-)

diff --git a/gdb/target.c b/gdb/target.c
index 34db652..bcc736b 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -432,6 +432,35 @@ target_load (const char *arg, int from_tty)
   (*current_target.to_load) (&current_target, arg, from_tty);
 }
 
+/* Possible terminal states.  */
+
+enum terminal_state
+  {
+    /* The inferior's terminal settings are in effect.  */
+    terminal_is_inferior = 0,
+
+    /* Some of our terminal settings are in effect, enough to get
+       proper output.  */
+    terminal_is_ours_for_output = 1,
+
+    /* Our terminal settings are in effect, for output and input.  */
+    terminal_is_ours = 2
+  };
+
+static enum terminal_state terminal_state;
+
+/* See target.h.  */
+
+void
+target_terminal_init (void)
+{
+  (*current_target.to_terminal_init) (&current_target);
+
+  terminal_state = terminal_is_ours;
+}
+
+/* See target.h.  */
+
 void
 target_terminal_inferior (void)
 {
@@ -442,9 +471,36 @@ target_terminal_inferior (void)
   if (target_can_async_p () && !sync_execution)
     return;
 
+  if (terminal_state == terminal_is_inferior)
+    return;
+
   /* If GDB is resuming the inferior in the foreground, install
      inferior's terminal modes.  */
   (*current_target.to_terminal_inferior) (&current_target);
+  terminal_state = terminal_is_inferior;
+}
+
+/* See target.h.  */
+
+void
+target_terminal_ours (void)
+{
+  if (terminal_state == terminal_is_ours)
+    return;
+
+  (*current_target.to_terminal_ours) (&current_target);
+  terminal_state = terminal_is_ours;
+}
+
+/* See target.h.  */
+
+void
+target_terminal_ours_for_output (void)
+{
+  if (terminal_state != terminal_is_inferior)
+    return;
+  (*current_target.to_terminal_ours_for_output) (&current_target);
+  terminal_state = terminal_is_ours_for_output;
 }
 
 /* See target.h.  */
diff --git a/gdb/target.h b/gdb/target.h
index a679228..c1ab124 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1383,31 +1383,25 @@ extern int target_remove_breakpoint (struct gdbarch *gdbarch,
 /* Initialize the terminal settings we record for the inferior,
    before we actually run the inferior.  */
 
-#define target_terminal_init() \
-     (*current_target.to_terminal_init) (&current_target)
+extern void target_terminal_init (void);
 
 /* Put the inferior's terminal settings into effect.
    This is preparation for starting or resuming the inferior.  */
 
 extern void target_terminal_inferior (void);
 
-/* Put some of our terminal settings into effect,
-   enough to get proper results from our output,
-   but do not change into or out of RAW mode
-   so that no input is discarded.
+/* Put some of our terminal settings into effect, enough to get proper
+   results from our output, but do not change into or out of RAW mode
+   so that no input is discarded.  This is a no-op if terminal_ours
+   was most recently called.  */
 
-   After doing this, either terminal_ours or terminal_inferior
-   should be called to get back to a normal state of affairs.  */
-
-#define target_terminal_ours_for_output() \
-     (*current_target.to_terminal_ours_for_output) (&current_target)
+extern void target_terminal_ours_for_output (void);
 
 /* Put our terminal settings into effect.
    First record the inferior's terminal settings
    so they can be restored properly later.  */
 
-#define target_terminal_ours() \
-     (*current_target.to_terminal_ours) (&current_target)
+extern void target_terminal_ours (void);
 
 /* Return true if the target stack has a non-default
   "to_terminal_ours" method.  */
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 754a2d1..0f98793 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1741,7 +1741,7 @@ do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching)
      current thread until we report an event out of windows_wait.  */
   inferior_ptid = pid_to_ptid (pid);
 
-  child_terminal_init_with_pgrp (pid);
+  target_terminal_init ();
   target_terminal_inferior ();
 
   windows_initialization_done = 0;
-- 
1.9.3

  parent reply	other threads:[~2014-10-09 18:00 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-09 18:00 [PATCH 0/4] Fix a set of terminal/readline handling bugs/crashes Pedro Alves
2014-10-09 18:00 ` [PATCH 3/4] PR gdb/17300: Input after "c -a" crashes readline/GDB Pedro Alves
2014-10-09 18:00 ` Pedro Alves [this message]
2014-10-09 18:00 ` [PATCH 2/4] PR gdb/17472: With annotations, input while executing in the foreground " Pedro Alves
2014-10-09 18:00 ` [PATCH 4/4] PR gdb/17471: Repeating a background command makes it foreground Pedro Alves
2015-08-03 21:02   ` [patch] ASAN attach crash - 7.9 regression [Re: [PATCH 4/4] PR gdb/17471: Repeating a background command makes it foreground] Jan Kratochvil
2015-08-04  8:35     ` Pedro Alves
2015-08-04 11:48       ` [commit+7.10] " Jan Kratochvil
2015-08-25 15:47         ` Jan Kratochvil
2015-08-04  8:28   ` [patch] signal_command: Leftover cleanup chain " Jan Kratochvil
2015-08-04  8:37     ` Pedro Alves
2015-08-04 11:49       ` [commit+7.10] " Jan Kratochvil
2015-08-25 15:48         ` Jan Kratochvil
2014-10-17 13:39 ` [pushed] Re: [PATCH 0/4] Fix a set of terminal/readline handling bugs/crashes Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1412877629-12052-2-git-send-email-palves@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).