Re: [RFA]: New function terminal_save_ours()

[Stephane Carrez <Stephane.Carrez@worldnet.fr>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 6753 bytes --]

Hi!

Andrew Cagney a écrit :
> 
> > Hi!
> >
> > When using gdb-tui and switching to or leaving the TUI mode, the terminal
> > settings are changed.  This is part of the curses management (endwin).
> >
> > Gdb keeps in a static variable in inflow.c for the tty setting of himself.
> > It switches it with the inferior terminal (terminal_inferior, terminal_ours).
> > The gdb terminal settings are fetched only once, the first time it is
> > used/checked (so we don't know when).
> >
> > For the TUI to work correctly, it needs to update the gdb knowledge of
> > its terminal.  The patch below proposes a new function `terminal_save_ours'
> > which is called by TUI each time the TUI changes from mode;  that is
> > after we enter in TUI mode (curses), and after we left the TUI mode.
> >
> > Can you approve this patch ?
> 
> I wish it were this simple.
> 
so do I...

> target_terminal_ours(), target_terminal_inferior() and
> target_terminal_ours_for_output() all go through the target vector.  The
> , er, obvious (?) thing to do would be to add
> target_terminal_save_our_tty_state(), or what ever, to that target
> vector.  I'm, to be honest, not sure if it is ``obvious'' or not.
> 
> Think about a remote target that has implemented console I/O.
> 
> When GDB hands off to the target it calls target_terminal_inferior(),
> that in turn causes remote.c to steal the input from GDB so that it can
> funnel all console input down to the remote target.  When the inferior
> stops, GDB calls target_terminal_ours(), and the target relinquishes
> that control.  At present, remote.c (and possibly all other remote
> targets) assumes that this operation doesn't require any get/set tty
> state manipulation.  I don't think the patch, as it currently stands
> addresses this.

Agree.  It does not address that.

> 
> How to best address it is where things get tricky.  I'm not sure if the
> targets should all be updated to do the set/get tty tweeking or if the
> set/get tty stuff should be brought closer to GDB and always done.
> 
>         Andrew
> 

Can we see this as a first step?

I would like to avoid to change the target vector before 5.1.
We can introduce the target function after 5.1 and let it point to
the terminal_save_our_state() as for other terminal_xxx().
The TUI uses terminal_save_our_state() and we will switch it
to the target_xxx().


> > Index: inflow.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/inflow.c,v
> > retrieving revision 1.11
> > diff -u -p -r1.11 inflow.c
> > --- inflow.c  2001/07/15 20:34:13     1.11
> > +++ inflow.c  2001/07/19 22:48:49
> > @@ -200,6 +200,23 @@ terminal_init_inferior_with_pgrp (int pg
> >      }
> >  }
> >
> > +/* Save the terminal settings again.  This is necessary for the TUI
> > +   when it switches to TUI or non-TUI mode;  curses changes the terminal
> > +   and gdb must be able to restore it correctly.  */
> > +
> > +void
> > +terminal_save_ours ()
> 
> I'd recommend a more explicit name - ..._save_our_state?
> 
Ok

> > +{
> > +  if (gdb_has_a_terminal ())
> > +    {
> > +      /* We could just as well copy our_ttystate (if we felt like adding
> > +         a new function serial_copy_tty_state).  */
> > +      if (our_ttystate)
> > +     xfree (our_ttystate);
> > +      our_ttystate = serial_get_tty_state (stdin_serial);
> > +    }
> > +}
> 
> Should all existing ``our_ttystate = serial_get_tty_state()'' calls be
> replaced with calls to this function?
> 
>         Andrew

There is only one such call and it is in gdb_has_a_terminal().
I don't see any benefit in doing so.

	Stephane

2001-07-24  Stephane Carrez  <Stephane.Carrez@worldnet.fr>

        * inferior.h (terminal_save_our_state): Declare.
        * inflow.c (terminal_save_our_state): New function.
Index: inflow.c
===================================================================
RCS file: /cvs/src/src/gdb/inflow.c,v
retrieving revision 1.11
diff -u -p -r1.11 inflow.c
--- inflow.c	2001/07/15 20:34:13	1.11
+++ inflow.c	2001/07/24 20:42:22
@@ -200,6 +200,23 @@ terminal_init_inferior_with_pgrp (int pg
     }
 }
 
+/* Save the terminal settings again.  This is necessary for the TUI
+   when it switches to TUI or non-TUI mode;  curses changes the terminal
+   and gdb must be able to restore it correctly.  */
+
+void
+terminal_save_our_state ()
+{
+  if (gdb_has_a_terminal ())
+    {
+      /* We could just as well copy our_ttystate (if we felt like adding
+         a new function serial_copy_tty_state).  */
+      if (our_ttystate)
+	xfree (our_ttystate);
+      our_ttystate = serial_get_tty_state (stdin_serial);
+    }
+}
+
 void
 terminal_init_inferior (void)
 {
Index: inferior.h
===================================================================
RCS file: /cvs/src/src/gdb/inferior.h,v
retrieving revision 1.23
diff -u -p -r1.23 inferior.h
--- inferior.h	2001/05/15 00:03:36	1.23
+++ inferior.h	2001/07/24 20:42:28
@@ -151,6 +151,8 @@ extern void generic_mourn_inferior (void
 
 extern void terminal_ours (void);
 
+extern void terminal_save_our_state (void);
+
 extern int run_stack_dummy (CORE_ADDR, char *);
 
 extern CORE_ADDR read_pc (void);
From ac131313@cygnus.com Tue Jul 24 14:09:00 2001
From: Andrew Cagney <ac131313@cygnus.com>
To: Stephane Carrez <Stephane.Carrez@worldnet.fr>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA]: New function terminal_save_ours()
Date: Tue, 24 Jul 2001 14:09:00 -0000
Message-id: <3B5DE3F1.8010605@cygnus.com>
References: <3B576530.13178D50@worldnet.fr> <3B5DABFE.2010209@cygnus.com> <3B5DDF72.3A59A477@worldnet.fr>
X-SW-Source: 2001-07/msg00606.html
Content-length: 1108

> 
> Agree.  It does not address that.
> 
> 
>> 
>> How to best address it is where things get tricky.  I'm not sure if the
>> targets should all be updated to do the set/get tty tweeking or if the
>> set/get tty stuff should be brought closer to GDB and always done.
>> 
>> Andrew
>> 
> 
> 
> Can we see this as a first step?
> 
> I would like to avoid to change the target vector before 5.1.
> We can introduce the target function after 5.1 and let it point to
> the terminal_save_our_state() as for other terminal_xxx().
> The TUI uses terminal_save_our_state() and we will switch it
> to the target_xxx().


Sorry, but no, I'd rather see the probem fixed :-/  You're just, 
unfortunatly, the one to draw the short straw.

The target vector has a very sordid history in this regard.  Too often 
have people allowed things to slip through that should have, instead, 
been included in the target vector.

For the TUI, I was thinking more of a 5.2 timeframe.  Regarding the 
target vector, I might cook something (gdbarch.sh like) up - but what 
ever it is it wouldn't go in until 5.1 is branched.

	Andrew