PATCH: MinGW readline -- revised

PATCH: MinGW readline -- revised

[Mark Mitchell]

This patch is the revised GDB-on-MinGW bit.

There are three subparts:

1) Minor bit-rot in remote-sim.c and ser-tcp.c on MinGW.  In
   particular, SIGTRAP is being used unconditionally in the former,
   and the MinGW definition of "close" (in terms of "closesocket")
   needs to be a function-like macro, so as to avoid confusion in
   code like "ops->close = net_close".

2) On the GDB side, I've added a win32-termcap.c file that contains
   the stub termcap implementation.  There seemed to be no consensus
   that it belongs in libiberty, so I've submitted it here.  I'd
   really appreciate being able to check this in; if it's later
   decided that it's wanted in libiberty, I'll happily move it.

3) On the readline side, I've backported the MinGW changes that will
   be in readline 5.1.

OK?

--
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com

2005-07-18  Mark Mitchell  <mark@codesourcery.com>

	* configure.ac: On MinGW, do not require a termcap library, and
	use win32-termcap.c.
	* configure: Regenerated.
	* win32-termcap.c: New file.

	* remote-sim.c (gdbsim_wait): Do not use SIGTRAP if not
	defined.

	* ser-tcp.c (close): Define as a function-like macro on MinGW.

2005-07-18  Mark Mitchell <mark@codesourcery.com>

	* input.c (rl_getc): Use getch to read console input on
	Windows.
	* readline.c (bind_arrow_keys_internal): Translate
	Windows keysequences into POSIX key sequences.
	* rldefs.h (NO_TTY_DRIVER): Define on MinGW.
	* rltty.c: Conditionalize on NO_TTY_DRIVER throughout.

Index: gdb/configure.ac
===================================================================
RCS file: /cvs/src/src/gdb/configure.ac,v
retrieving revision 1.23
diff -c -5 -p -r1.23 configure.ac
*** gdb/configure.ac	3 Jul 2005 16:05:07 -0000	1.23
--- gdb/configure.ac	19 Jul 2005 00:05:26 -0000
*************** case $host_os in
*** 330,339 ****
--- 330,343 ----
        ac_cv_search_tgetent="../libtermcap/libtermcap.a"
      fi ;;
    go32* | *djgpp*)
      ac_cv_search_tgetent="none required"
      ;;
+   *mingw32*)	 
+     ac_cv_search_tgetent="none required"
+     CONFIG_OBS="$CONFIG_OBS win32-termcap.o"
+     ;;
  esac
  
  # These are the libraries checked by Readline.
  AC_SEARCH_LIBS(tgetent, [termcap tinfo curses ncurses])

Index: gdb/remote-sim.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-sim.c,v
retrieving revision 1.49
diff -c -5 -p -r1.49 remote-sim.c
*** gdb/remote-sim.c	19 Jun 2005 20:08:37 -0000	1.49
--- gdb/remote-sim.c	18 Jul 2005 23:59:21 -0000
*************** gdbsim_wait (ptid_t ptid, struct target_
*** 692,702 ****
--- 692,704 ----
  	{
  	case SIGABRT:
  	  quit ();
  	  break;
  	case SIGINT:
+ #ifdef SIGTRAP
  	case SIGTRAP:
+ #endif
  	default:
  	  status->kind = TARGET_WAITKIND_STOPPED;
  	  /* The signal in sigrc is a host signal.  That probably
  	     should be fixed.  */
  	  status->value.sig = target_signal_from_host (sigrc);
Index: gdb/ser-tcp.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-tcp.c,v
retrieving revision 1.23
diff -c -5 -p -r1.23 ser-tcp.c
*** gdb/ser-tcp.c	13 Jun 2005 21:31:57 -0000	1.23
--- gdb/ser-tcp.c	18 Jul 2005 23:59:21 -0000
***************
*** 37,47 ****
  #include <sys/time.h>
  
  #ifdef USE_WIN32API
  #include <winsock2.h>
  #define ETIMEDOUT WSAETIMEDOUT
! #define close closesocket
  #define ioctl ioctlsocket
  #else
  #include <netinet/in.h>
  #include <arpa/inet.h>
  #include <netdb.h>
--- 37,47 ----
  #include <sys/time.h>
  
  #ifdef USE_WIN32API
  #include <winsock2.h>
  #define ETIMEDOUT WSAETIMEDOUT
! #define close(fd) closesocket (fd)
  #define ioctl ioctlsocket
  #else
  #include <netinet/in.h>
  #include <arpa/inet.h>
  #include <netdb.h>
Index: gdb/win32-termcap.c
===================================================================
RCS file: gdb/win32-termcap.c
diff -N gdb/win32-termcap.c
*** /dev/null	1 Jan 1970 00:00:00 -0000
--- gdb/win32-termcap.c	18 Jul 2005 23:59:21 -0000
***************
*** 0 ****
--- 1,65 ----
+ /* Win32 termcap emulation.
+ 
+    Copyright 2005 Free Software Foundation, Inc.
+ 
+    Contributed by CodeSourcery, LLC.
+ 
+    This file is part of GDB.
+ 
+    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 2 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 eve nthe 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, write to the Free Software
+    Foundation, Inc., 51 Franklin St., Fifth Floor, Boston, MA
+    02110-1301, USA.  */
+ 
+ #include <stdlib.h>
+ 
+ /* Each of the files below is a minimal implementation of the standard
+    termcap function with the same name, suitable for use in a Windows
+    console window.  */
+ 
+ int
+ tgetent (char *buffer, char *termtype)
+ {
+   return -1;
+ }
+ 
+ int
+ tgetnum (char *name)
+ {
+   return -1;
+ }
+ 
+ int
+ tgetflag (char *name)
+ {
+   return -1;
+ }
+ 
+ char *
+ tgetstr (char *name, char **area)
+ {
+   return NULL;
+ }
+ 
+ int
+ tputs (char *string, int nlines, int (*outfun) ())
+ {
+   while (*string)
+     outfun (*string++);
+ }
+ 
+ char *
+ tgoto (const char *cap, int col, int row)
+ {
+   return NULL;
+ }
Index: readline/input.c
===================================================================
RCS file: /cvs/src/src/readline/input.c,v
retrieving revision 1.5
diff -c -5 -p -r1.5 input.c
*** readline/input.c	8 Dec 2002 22:31:37 -0000	1.5
--- readline/input.c	18 Jul 2005 23:59:21 -0000
*************** rl_getc (stream)
*** 422,431 ****
--- 422,438 ----
    int result;
    unsigned char c;
  
    while (1)
      {
+ #ifdef __MINGW32__
+       /* On Windows, use a special routine to read a single  character
+ 	 from the console.  (Otherwise, no characters are available
+ 	 until the user hits the return key.)  */
+       if (isatty (fileno (stream)))
+ 	return getch ();
+ #endif
        result = read (fileno (stream), &c, sizeof (unsigned char));
  
        if (result == sizeof (unsigned char))
  	return (c);
  
Index: readline/readline.c
===================================================================
RCS file: /cvs/src/src/readline/readline.c,v
retrieving revision 1.7
diff -c -5 -p -r1.7 readline.c
*** readline/readline.c	27 Jan 2004 22:25:15 -0000	1.7
--- readline/readline.c	18 Jul 2005 23:59:21 -0000
*************** bind_arrow_keys_internal (map)
*** 866,875 ****
--- 866,891 ----
     _rl_bind_if_unbound ("\033[0B", rl_backward_char);
     _rl_bind_if_unbound ("\033[0C", rl_forward_char);
     _rl_bind_if_unbound ("\033[0D", rl_get_next_history);
  #endif
  
+ #ifdef __MINGW32__
+    /* Under Windows, when an extend key (like an arrow key) is
+       pressed, getch() will return 340 (octal) followed by a code for
+       the extended key.  We use macros to transform those into the
+       normal ANSI terminal sequences for these keys.  */
+ 
+    /* Up arrow.  */
+    rl_macro_bind ("\340H", "\033[A", map);
+    /* Left arrow.  */
+    rl_macro_bind ("\340K", "\033[D", map);
+    /* Right arrow.  */
+    rl_macro_bind ("\340M", "\033[C", map);
+    /* Down arrow.  */
+    rl_macro_bind ("\340P", "\033[B", map);
+ #endif
+ 
    _rl_bind_if_unbound ("\033[A", rl_get_previous_history);
    _rl_bind_if_unbound ("\033[B", rl_get_next_history);
    _rl_bind_if_unbound ("\033[C", rl_forward_char);
    _rl_bind_if_unbound ("\033[D", rl_backward_char);
    _rl_bind_if_unbound ("\033[H", rl_beg_of_line);
Index: readline/rldefs.h
===================================================================
RCS file: /cvs/src/src/readline/rldefs.h,v
retrieving revision 1.4
diff -c -5 -p -r1.4 rldefs.h
*** readline/rldefs.h	8 Dec 2002 22:31:37 -0000	1.4
--- readline/rldefs.h	18 Jul 2005 23:59:21 -0000
***************
*** 30,40 ****
  #  include "config.h"
  #endif
  
  #include "rlstdc.h"
  
! #if defined (_POSIX_VERSION) && !defined (TERMIOS_MISSING)
  #  define TERMIOS_TTY_DRIVER
  #else
  #  if defined (HAVE_TERMIO_H)
  #    define TERMIO_TTY_DRIVER
  #  else
--- 30,42 ----
  #  include "config.h"
  #endif
  
  #include "rlstdc.h"
  
! #if defined (__MINGW32__)
! #  define NO_TTY_DRIVER
! #elif defined (_POSIX_VERSION) && !defined (TERMIOS_MISSING)
  #  define TERMIOS_TTY_DRIVER
  #else
  #  if defined (HAVE_TERMIO_H)
  #    define TERMIO_TTY_DRIVER
  #  else
Index: readline/rltty.c
===================================================================
RCS file: /cvs/src/src/readline/rltty.c,v
retrieving revision 1.6
diff -c -5 -p -r1.6 rltty.c
*** readline/rltty.c	8 Dec 2002 22:31:37 -0000	1.6
--- readline/rltty.c	18 Jul 2005 23:59:21 -0000
*************** set_winsize (tty)
*** 150,160 ****
    if (ioctl (tty, TIOCGWINSZ, &w) == 0)
        (void) ioctl (tty, TIOCSWINSZ, &w);
  #endif /* TIOCGWINSZ */
  }
  
! #if defined (NEW_TTY_DRIVER)
  
  /* Values for the `flags' field of a struct bsdtty.  This tells which
     elements of the struct bsdtty have been fetched from the system and
     are valid. */
  #define SGTTY_SET	0x01
--- 150,162 ----
    if (ioctl (tty, TIOCGWINSZ, &w) == 0)
        (void) ioctl (tty, TIOCSWINSZ, &w);
  #endif /* TIOCGWINSZ */
  }
  
! #if defined (NO_TTY_DRIVER)
! /* Nothing */
! #elif defined (NEW_TTY_DRIVER)
  
  /* Values for the `flags' field of a struct bsdtty.  This tells which
     elements of the struct bsdtty have been fetched from the system and
     are valid. */
  #define SGTTY_SET	0x01
*************** prepare_terminal_settings (meta_flag, ol
*** 630,639 ****
--- 632,657 ----
  
  #endif /* TERMIOS_TTY_DRIVER && _POSIX_VDISABLE */
  }
  #endif  /* NEW_TTY_DRIVER */
  
+ /* Put the terminal in CBREAK mode so that we can detect key
+    presses. */
+ #if defined (NO_TTY_DRIVER)
+ void
+ rl_prep_terminal (meta_flag)
+      int meta_flag;
+ {
+   readline_echoing_p = 1;
+ }
+ 
+ void
+ rl_deprep_terminal ()
+ {
+ }
+ 
+ #else /* ! NO_TTY_DRIVER */
  /* Put the terminal in CBREAK mode so that we can detect key presses. */
  void
  rl_prep_terminal (meta_flag)
       int meta_flag;
  {
*************** rl_deprep_terminal ()
*** 704,713 ****
--- 722,732 ----
    terminal_prepped = 0;
    RL_UNSETSTATE(RL_STATE_TERMPREPPED);
  
    release_sigint ();
  }
+ #endif /* !NO_TTY_DRIVER */
  \f
  /* **************************************************************** */
  /*								    */
  /*			Bogus Flow Control      		    */
  /*								    */
*************** rl_deprep_terminal ()
*** 715,724 ****
--- 734,747 ----
  
  int
  rl_restart_output (count, key)
       int count, key;
  {
+ #if defined (__MINGW32__)
+   return 0;
+ #else /* !__MING32__ */
+ 
    int fildes = fileno (rl_outstream);
  #if defined (TIOCSTART)
  #if defined (apollo)
    ioctl (&fildes, TIOCSTART, 0);
  #else
*************** rl_restart_output (count, key)
*** 742,757 ****
--- 765,785 ----
  #    endif /* TCXONC */
  #  endif /* !TERMIOS_TTY_DRIVER */
  #endif /* !TIOCSTART */
  
    return 0;
+ #endif /* !__MINGW32__ */
  }
  
  int
  rl_stop_output (count, key)
       int count, key;
  {
+ #if defined (__MINGW32__)
+   return 0;
+ #else
+ 
    int fildes = fileno (rl_instream);
  
  #if defined (TIOCSTOP)
  # if defined (apollo)
    ioctl (&fildes, TIOCSTOP, 0);
*************** rl_stop_output (count, key)
*** 770,779 ****
--- 798,808 ----
  #   endif /* TCXONC */
  # endif /* !TERMIOS_TTY_DRIVER */
  #endif /* !TIOCSTOP */
  
    return 0;
+ #endif /* !__MINGW32__ */
  }
  
  /* **************************************************************** */
  /*								    */
  /*			Default Key Bindings			    */
*************** rl_stop_output (count, key)
*** 784,793 ****
--- 813,823 ----
     in KMAP.  Should be static, now that we have rl_tty_set_default_bindings. */
  void
  rltty_set_default_bindings (kmap)
       Keymap kmap;
  {
+ #if !defined (NO_TTY_DRIVER)
    TIOTYPE ttybuff;
    int tty = fileno (rl_instream);
  
  #if defined (NEW_TTY_DRIVER)
  
*************** rltty_set_default_bindings (kmap)
*** 842,851 ****
--- 872,882 ----
  #  if defined (VWERASE) && defined (TERMIOS_TTY_DRIVER)
        SET_SPECIAL (VWERASE, rl_unix_word_rubout);
  #  endif /* VWERASE && TERMIOS_TTY_DRIVER */
      }
  #endif /* !NEW_TTY_DRIVER */
+ #endif
  }
  
  /* New public way to set the system default editing chars to their readline
     equivalents. */
  void
*************** rl_tty_set_default_bindings (kmap)
*** 855,865 ****
    rltty_set_default_bindings (kmap);
  }
  
  #if defined (HANDLE_SIGNALS)
  
! #if defined (NEW_TTY_DRIVER)
  int
  _rl_disable_tty_signals ()
  {
    return 0;
  }
--- 886,896 ----
    rltty_set_default_bindings (kmap);
  }
  
  #if defined (HANDLE_SIGNALS)
  
! #if defined (NEW_TTY_DRIVER) || defined (NO_TTY_DRIVER)
  int
  _rl_disable_tty_signals ()
  {
    return 0;
  }

Re: PATCH: MinGW readline -- revised

[Kai Henningsen]

mark@codesourcery.com (Mark Mitchell)  wrote on 18.07.05 in <200507190011.j6J0B1Ma014410@sethra.codesourcery.com>:

> 	* readline.c (bind_arrow_keys_internal): Translate
> 	Windows keysequences into POSIX key sequences.

s/POSIX/ANSI/



MfG Kai

Re: PATCH: MinGW readline -- revised

[Daniel Jacobowitz]

On Mon, Jul 18, 2005 at 05:11:01PM -0700, Mark Mitchell wrote:
> 
> This patch is the revised GDB-on-MinGW bit.
> 
> There are three subparts:

Unfortunately, this is a good example of why unrelated fixes
shouldn't be combined...

> 1) Minor bit-rot in remote-sim.c and ser-tcp.c on MinGW.  In
>    particular, SIGTRAP is being used unconditionally in the former,
>    and the MinGW definition of "close" (in terms of "closesocket")
>    needs to be a function-like macro, so as to avoid confusion in
>    code like "ops->close = net_close".

I'm not sure why you call this "bit-rot".

The unconditional use of SIGTRAP has been there since the oldest
version of GDB in CVS.  Paul posted a patch to provide a default
definition of SIGTRAP instead in two places; his patch is more correct
than yours, and is on the csl-arm branch.  See the simulator sources to
understand why you can't just ignore SIGTRAP here; the simulator will
be completely broken by this change.  Paul's approach is still somewhat
incorrect, in that there is no valid excuse for using native signal
numbers here.  The simulators really need to be fixed, but it would be
a Herculean effort.  I am inclined to go with Paul's patch for the
nonce.

Similarly the reference to ops->close is from 2002.

> 2) On the GDB side, I've added a win32-termcap.c file that contains
>    the stub termcap implementation.  There seemed to be no consensus
>    that it belongs in libiberty, so I've submitted it here.  I'd
>    really appreciate being able to check this in; if it's later
>    decided that it's wanted in libiberty, I'll happily move it.
> 
> 3) On the readline side, I've backported the MinGW changes that will
>    be in readline 5.1.

These look fine; the changes aren't quite the way I'd have liked them,
but if they've been taken for readline 5.1, it's important that we
minimize divergence.  Chris, do these parts look OK to you?

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: MinGW readline -- revised

[Mark Mitchell]

Daniel Jacobowitz wrote:

>>1) Minor bit-rot in remote-sim.c and ser-tcp.c on MinGW.  In
>>   particular, SIGTRAP is being used unconditionally in the former,
>>   and the MinGW definition of "close" (in terms of "closesocket")
>>   needs to be a function-like macro, so as to avoid confusion in
>>   code like "ops->close = net_close".
>  
> I'm not sure why you call this "bit-rot".

This configuration worked in the past for me, but does not work now. 
Perhaps that's due to something in my environment that has changed, or 
use of some other patch to the source code I was testing, or a different 
--target setting, or something.  In any case, I hereby withdraw the use 
of the word bit-rot!

> I am inclined to go with Paul's patch for the nonce.

OK; consider that hunk withrdrawn until we figure out what to do instead.

> Similarly the reference to ops->close is from 2002.

Independently of that, may I check in the change that fixes it?  This 
change is entirely within MinGW-#ifdef'd code, and is purely syntactic 
in nature.

For clarity, here is the patch:

2005-07-18  Mark Mitchell  <mark@codesourcery.com>

	* ser-tcp.c (close): Define as a function-like macro on MinGW.

Index: ser-tcp.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-tcp.c,v
retrieving revision 1.23
diff -c -5 -p -r1.23 ser-tcp.c
*** ser-tcp.c   13 Jun 2005 21:31:57 -0000      1.23
--- ser-tcp.c   24 Jul 2005 22:45:25 -0000
***************
*** 37,47 ****
   #include <sys/time.h>

   #ifdef USE_WIN32API
   #include <winsock2.h>
   #define ETIMEDOUT WSAETIMEDOUT
! #define close closesocket
   #define ioctl ioctlsocket
   #else
   #include <netinet/in.h>
   #include <arpa/inet.h>
   #include <netdb.h>
--- 37,47 ----
   #include <sys/time.h>

   #ifdef USE_WIN32API
   #include <winsock2.h>
   #define ETIMEDOUT WSAETIMEDOUT
! #define close(fd) closesocket (fd)
   #define ioctl ioctlsocket
   #else
   #include <netinet/in.h>
   #include <arpa/inet.h>
   #include <netdb.h>

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: MinGW readline -- revised

[Daniel Jacobowitz]

On Sun, Jul 24, 2005 at 03:50:19PM -0700, Mark Mitchell wrote:
> Independently of that, may I check in the change that fixes it?  This 
> change is entirely within MinGW-#ifdef'd code, and is purely syntactic 
> in nature.
> 
> For clarity, here is the patch:
> 
> 2005-07-18  Mark Mitchell  <mark@codesourcery.com>
> 
> 	* ser-tcp.c (close): Define as a function-like macro on MinGW.

Sure.  This is OK.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: MinGW readline -- revised

[Mark Mitchell]

Daniel Jacobowitz wrote:
> On Sun, Jul 24, 2005 at 03:50:19PM -0700, Mark Mitchell wrote:
> 
>>Independently of that, may I check in the change that fixes it?  This 
>>change is entirely within MinGW-#ifdef'd code, and is purely syntactic 
>>in nature.
>>
>>For clarity, here is the patch:
>>
>>2005-07-18  Mark Mitchell  <mark@codesourcery.com>
>>
>>	* ser-tcp.c (close): Define as a function-like macro on MinGW.
> 
> 
> Sure.  This is OK.

Committed, thanks.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: MinGW readline -- revised

[Christopher Faylor]

On Sun, Jul 24, 2005 at 05:10:16PM -0400, Daniel Jacobowitz wrote:
>These look fine; the changes aren't quite the way I'd have liked them,
>but if they've been taken for readline 5.1, it's important that we
>minimize divergence.  Chris, do these parts look OK to you?

*** readline/input.c    8 Dec 2002 22:31:37 -0000       1.5
--- readline/input.c    18 Jul 2005 23:59:21 -0000
*************** rl_getc (stream)
*** 422,431 ****
--- 422,438 ----
    int result;
    unsigned char c;

    while (1)
      {
+ #ifdef __MINGW32__
+       /* On Windows, use a special routine to read a single  character
+        from the console.  (Otherwise, no characters are available
+        until the user hits the return key.)  */
+       if (isatty (fileno (stream)))
+       return getch ();
+ #endif
        result = read (fileno (stream), &c, sizeof (unsigned char));

        if (result == sizeof (unsigned char))
        return (c);


This doesn't look right.  Shouldn't there be an ifdef there?  It's a
minor point but it looks like this would potentially produce dead code.

Other than that I have no objections other than to add an obligatory
grumble about the need to use a getch windows-ism.

cgf

Re: PATCH: MinGW readline -- revised

[Daniel Jacobowitz]

On Sun, Jul 24, 2005 at 08:39:30PM -0400, Christopher Faylor wrote:
> + #ifdef __MINGW32__
> +       /* On Windows, use a special routine to read a single  character
> +        from the console.  (Otherwise, no characters are available
> +        until the user hits the return key.)  */
> +       if (isatty (fileno (stream)))
> +       return getch ();
> + #endif

> This doesn't look right.  Shouldn't there be an ifdef there?  It's a
> minor point but it looks like this would potentially produce dead code.

I'm not quite sure what you mean - but if you're talking about the code
after the return statement, it looks like something's gone wrong with
the indendation in this bit.  Getch is only for consoles.

> Other than that I have no objections other than to add an obligatory
> grumble about the need to use a getch windows-ism.

Thanks!  Ditto.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: MinGW readline -- revised

[Mark Mitchell]

Christopher Faylor wrote:

Thanks for the review.

> *** readline/input.c    8 Dec 2002 22:31:37 -0000       1.5
> --- readline/input.c    18 Jul 2005 23:59:21 -0000
> *************** rl_getc (stream)
> *** 422,431 ****
> --- 422,438 ----
>     int result;
>     unsigned char c;
> 
>     while (1)
>       {
> + #ifdef __MINGW32__
> +       /* On Windows, use a special routine to read a single  character
> +        from the console.  (Otherwise, no characters are available
> +        until the user hits the return key.)  */
> +       if (isatty (fileno (stream)))
> +       return getch ();
> + #endif
>         result = read (fileno (stream), &c, sizeof (unsigned char));
> 
>         if (result == sizeof (unsigned char))
>         return (c);
> 
> 
> This doesn't look right.  Shouldn't there be an ifdef there?  It's a
> minor point but it looks like this would potentially produce dead code.

If the stream is *not* a TTY, then we still want to use the ordinary 
code.  There's just this one special case for TTYs.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: MinGW readline -- revised

[Christopher Faylor]

On Sun, Jul 24, 2005 at 08:41:09PM -0400, Daniel Jacobowitz wrote:
>On Sun, Jul 24, 2005 at 08:39:30PM -0400, Christopher Faylor wrote:
>> + #ifdef __MINGW32__
>> +       /* On Windows, use a special routine to read a single  character
>> +        from the console.  (Otherwise, no characters are available
>> +        until the user hits the return key.)  */
>> +       if (isatty (fileno (stream)))
>> +       return getch ();
>> + #endif
>
>> This doesn't look right.  Shouldn't there be an ifdef there?  It's a
>> minor point but it looks like this would potentially produce dead code.
>
>I'm not quite sure what you mean - but if you're talking about the code
>after the return statement, it looks like something's gone wrong with
>the indendation in this bit.  Getch is only for consoles.

I forgot that getch was only for consoles.  Somewhow I thought it could be
used for any stdin.  Silly me.  Windows.

cgf

Re: PATCH: MinGW readline -- revised

[Mark Mitchell]

Christopher Faylor wrote:
> On Sun, Jul 24, 2005 at 08:41:09PM -0400, Daniel Jacobowitz wrote:
> 
>>On Sun, Jul 24, 2005 at 08:39:30PM -0400, Christopher Faylor wrote:
>>
>>>+ #ifdef __MINGW32__
>>>+       /* On Windows, use a special routine to read a single  character
>>>+        from the console.  (Otherwise, no characters are available
>>>+        until the user hits the return key.)  */
>>>+       if (isatty (fileno (stream)))
>>>+       return getch ();
>>>+ #endif
>>
>>>This doesn't look right.  Shouldn't there be an ifdef there?  It's a
>>>minor point but it looks like this would potentially produce dead code.
>>
>>I'm not quite sure what you mean - but if you're talking about the code
>>after the return statement, it looks like something's gone wrong with
>>the indendation in this bit.  Getch is only for consoles.
> 
> 
> I forgot that getch was only for consoles.  Somewhow I thought it could be
> used for any stdin.  Silly me.  Windows.

So, is the patch OK?

Thanks,

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: MinGW readline -- revised

[Daniel Jacobowitz]

On Mon, Jul 25, 2005 at 07:55:27AM -0700, Mark Mitchell wrote:
> Christopher Faylor wrote:
> >On Sun, Jul 24, 2005 at 08:41:09PM -0400, Daniel Jacobowitz wrote:
> >
> >>On Sun, Jul 24, 2005 at 08:39:30PM -0400, Christopher Faylor wrote:
> >>
> >>>+ #ifdef __MINGW32__
> >>>+       /* On Windows, use a special routine to read a single  character
> >>>+        from the console.  (Otherwise, no characters are available
> >>>+        until the user hits the return key.)  */
> >>>+       if (isatty (fileno (stream)))
> >>>+       return getch ();
> >>>+ #endif
> >>
> >>>This doesn't look right.  Shouldn't there be an ifdef there?  It's a
> >>>minor point but it looks like this would potentially produce dead code.
> >>
> >>I'm not quite sure what you mean - but if you're talking about the code
> >>after the return statement, it looks like something's gone wrong with
> >>the indendation in this bit.  Getch is only for consoles.
> >
> >
> >I forgot that getch was only for consoles.  Somewhow I thought it could be
> >used for any stdin.  Silly me.  Windows.
> 
> So, is the patch OK?

Yes, the readline and win32-termcap parts are OK.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: MinGW readline -- revised

[Mark Mitchell]

Daniel Jacobowitz wrote:

> Yes, the readline and win32-termcap parts are OK.

Chris, Daniel --

Whee!  Thanks!

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: MinGW readline -- revised

[Mark Mitchell]

Daniel Jacobowitz wrote:

> Yes, the readline and win32-termcap parts are OK.

Committed.  (Offline, Daniel confirmed that the configure.ac change to 
gdb/ was OK as well.)

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304