This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Add comments to terminal_ours/inferior functions


Hi.

inflow.c has this:

/* 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.

   After doing this, either terminal_ours or terminal_inferior
   should be called to get back to a normal state of affairs.  */

void
child_terminal_ours_for_output (struct target_ops *self)
{
  child_terminal_ours_1 (1);
}

/* Put our terminal settings into effect.
   First record the inferior's terminal settings
   so they can be restored properly later.  */

void
child_terminal_ours (struct target_ops *self)
{
  child_terminal_ours_1 (0);
}

which seems ok (ie. to the reader the distinction between "terminal-ours" and
"terminal-ours-for-output" is properly handled) ... until you read
child_terminal_ours_1 where you find out the output_only distinction
is ignored.  Eh?

/* output_only is not used, and should not be used unless we introduce
   separate terminal_is_ours and terminal_is_ours_for_output
   flags.  */

static void
child_terminal_ours_1 (int output_only)
{
  ...
}

This is handled/worked-around in the linux implementation.
[One could say inf-child.c doesn't handle target-async and leaves it
to the target to subclass - e.g. inf-child.c has this:
  t->to_can_async_p = return_zero;]

/* target_terminal_ours implementation.  */

static void
linux_nat_terminal_ours (struct target_ops *self)
{
  if (!target_is_async_p ())
    {
      /* Async mode is disabled.  */
      child_terminal_ours (self);
      return;
    }

  /* GDB should never give the terminal to the inferior if the
     inferior is running in the background (run&, continue&, etc.),
     but claiming it sure should.  */
  child_terminal_ours (self);

  if (async_terminal_is_ours)
    return;

  clear_sigint_trap ();
  add_file_handler (input_fd, stdin_event_handler, 0);
  async_terminal_is_ours = 1;
}

I have a patch-in-progress to add async support to child_terminal_*,
but it's going to take some work before I think it's ready (still not
sure whether to do it at all, but it's interesting to play with).
In the interim I'd like to make the current code easier to
read/understand for the next time I pass through here.

2014-07-30  Doug Evans  <dje@google.com>

	* inflow.c (child_terminal_inferior): Add comment.
	(child_terminal_ours_for_output): Add comment.
	(child_terminal_ours): Add comment.
	* linux-nat.c (linux_nat_terminal_inferior): Add comment.
	(linux_nat_terminal_ours): Add comment.

diff --git a/gdb/inflow.c b/gdb/inflow.c
index 5f81de2..8d07757 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -267,7 +267,11 @@ child_terminal_init (struct target_ops *self)
 }
 
 /* Put the inferior's terminal settings into effect.
-   This is preparation for starting or resuming the inferior.  */
+   This is preparation for starting or resuming the inferior.
+
+   N.B. Targets that want to use this with async support must build that
+   support on top of this (e.g., the caller still needs to add stdin to the
+   event loop).  E.g., see linux_nat_terminal_inferior.  */
 
 void
 child_terminal_inferior (struct target_ops *self)
@@ -348,7 +352,10 @@ child_terminal_inferior (struct target_ops *self)
    so that no input is discarded.
 
    After doing this, either terminal_ours or terminal_inferior
-   should be called to get back to a normal state of affairs.  */
+   should be called to get back to a normal state of affairs.
+
+   N.B. The implementation is (currently) no different than
+   child_terminal_ours.  See child_terminal_ours_1.  */
 
 void
 child_terminal_ours_for_output (struct target_ops *self)
@@ -358,7 +365,11 @@ child_terminal_ours_for_output (struct target_ops *self)
 
 /* Put our terminal settings into effect.
    First record the inferior's terminal settings
-   so they can be restored properly later.  */
+   so they can be restored properly later.
+
+   N.B. Targets that want to use this with async support must build that
+   support on top of this (e.g., the caller still needs to add stdin to the
+   event loop).  E.g., see linux_nat_terminal_ours.  */
 
 void
 child_terminal_ours (struct target_ops *self)
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 8d4251f..5a791bc 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4595,7 +4595,9 @@ linux_nat_supports_disable_randomization (struct target_ops *self)
 
 static int async_terminal_is_ours = 1;
 
-/* target_terminal_inferior implementation.  */
+/* target_terminal_inferior implementation.
+
+   This is a wrapper around child_terminal_inferior to add async support.  */
 
 static void
 linux_nat_terminal_inferior (struct target_ops *self)
@@ -4618,7 +4620,14 @@ linux_nat_terminal_inferior (struct target_ops *self)
   set_sigint_trap ();
 }
 
-/* target_terminal_ours implementation.  */
+/* target_terminal_ours implementation.
+
+   This is a wrapper around child_terminal_ours to add async support (and
+   implement the target_terminal_ours vs target_terminal_ours_for_output
+   distinction).  child_terminal_ours is currently no different than
+   child_terminal_ours_for_output.
+   We leave target_terminal_ours_for_output alone, leaving it to
+   child_terminal_ours_for_output.  */
 
 static void
 linux_nat_terminal_ours (struct target_ops *self)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]