public inbox for cygwin-developers@cygwin.com
 help / color / mirror / Atom feed
* winpty injection
@ 2018-04-04 17:07 Thomas Wolff
  2018-04-04 22:04 ` spawn - where can I get it Mark A. Engel
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Thomas Wolff @ 2018-04-04 17:07 UTC (permalink / raw)
  To: cygwin-developers

[-- Attachment #1: Type: text/plain, Size: 1733 bytes --]

The interworking problems of pty-based terminals with native Windows 
programs have long been discussed and raise recurring user reports and 
complaints. Mintty issues https://github.com/mintty/mintty/issues/56 
(Improve support for native console programs) and 
https://github.com/mintty/mintty/issues/376 (Use different encodings for 
native Windows commands output and remote output) have dozens of duplicates.
There is the fairly well-known wrapper winpty which solves most of the 
interworking problems. However, it needs to be involved explicitly and 
selectively for Windows command-line applications.

So I had the idea that cygwin could apply winpty wrapping automatically 
when it detects that a non-cygwin command-line program is being called. 
The attached patch demonstrates how this would work. It is a 
proof-of-concept, not ready for integration but provided for discussion 
of the approach (thus not sent to cygwin-patches).
If it is appreciated, there are a number of open issues:
* The patch offers two methods. The activated one (#ifdef 
inject_winpty_recursively) works but leaves a memory leak in the argv1 
array needed to expand the argv with the injected wrapper. The other 
one, which tries to inject winpty into argv and then continue the 
function linearly, would solve the memory leak but does not work (yet)...
* The winpty tool is not even a cygwin package yet. If the injection 
approach is approved, it should be packaged as a base package, or even 
its code could be migrated into cygwin.
* As some people may object such a magic (and it might in fact raise 
unexpected problems), it could also be made dependent on a setting in 
the CYGWIN environment variable.

Looking forward to feedback
Thomas

[-- Attachment #2: 0000-winpty-injection-proof-of-concept.patch --]
[-- Type: text/plain, Size: 3338 bytes --]

From 2233529613af5a3b569a078d4e33334bbf8bd7e2 Mon Sep 17 00:00:00 2001
From: Thomas Wolff <towo@towo.net>
Date: Fri, 23 Mar 2018 22:34:11 +0100
Subject: [PATCH] winpty injection (proof of concept)

If a non-cygwin program is run from a pty-based environment,
the winpty wrapper is injected to adapt:
* pty handling, especially raw input
* character set conversion
* limited signal handling interworking
---
 winsup/cygwin/spawn.cc | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 37db526..3cd442e 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -292,6 +292,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
   av newargv;
   linebuf cmd;
   PWCHAR envblock = NULL;
+  char * * argv1 = 0;
   path_conv real_path;
   bool reset_sendsig = false;
 
@@ -332,10 +333,53 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
 	}
 
       res = newargv.setup (prog_arg, real_path, ext, ac, argv, p_type_exec);
-
       if (res)
 	__leave;
 
+      /* winpty injection */
+
+      // provide a check for proper installation of winpty;
+      // this slows us down a bit (thus called last below),
+      // maybe it could check winpty.exe only or nothing...
+#define winpty_available()	\
+		   !access ("/bin/winpty.exe", R_OK | X_OK) \
+		&& !access ("/bin/winpty.dll", R_OK | X_OK) \
+		&& !access ("/bin/winpty-agent.exe", R_OK | X_OK)
+
+      bool ispty = !strncmp (ttyname (0), "/dev/pty", 8);
+      bool isgui = false;
+      // we could check for a GUI program, as determined by `file` ("(GUI)"),
+      // and possibly others (not "(console)"), which do not need the wrapper
+      debug_printf("winpty injection check %s cyg %d pty %d\n", prog_arg, real_path.iscygexec (), ispty);
+      if (!real_path.iscygexec () && ispty && !isgui && winpty_available ())
+	{
+	  debug_printf("argv1 allocating\n");
+	  argv1 = (char**) malloc (sizeof(char*) * (ac + 2));
+	  for (ac = 0; argv[ac]; ac++)
+	    argv1[ac + 1] = (char*) argv[ac];
+	  argv1[ac + 1] = 0;
+	  argv1[0] = (char*) prog_arg;
+
+#define inject_winpty_recursively
+
+#ifdef inject_winpty_recursively
+	  res = worker ("/bin/winpty", argv1, envp, mode, in__stdin, in__stdout);
+	  debug_printf("worker -> %d\n", res);
+	  // if we return here, probably winpty was not found, so we could 
+	  // try to continue and skip __leave, but that does not work
+	  __leave;
+#else
+	  /* repeat newargv setup above */
+	  // this method does not work, somehow the parameters are mangled
+	  // but it would resolve the argv1 memory leak
+	  res = newargv.setup ("/bin/winpty", real_path, ext, ac, argv1, p_type_exec);
+	  free (argv1); argv1 = 0; // with this method, drop free() below
+	  if (res)
+	    __leave;
+	  debug_printf("continuing after argv1 adjustment\n");
+#endif
+	}
+
       if (!real_path.iscygexec () && ::cygheap->cwd.get_error ())
 	{
 	  small_printf ("Error: Current working directory %s.\n"
@@ -846,6 +890,10 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
   this->cleanup ();
   if (envblock)
     free (envblock);
+  if (argv1) {
+    debug_printf("argv1 freeing\n", argv1);
+    free (argv1);
+  }
   return (int) res;
 }
 
-- 
2.16.2


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-08-16 10:36 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-04 17:07 winpty injection Thomas Wolff
2018-04-04 22:04 ` spawn - where can I get it Mark A. Engel
2018-04-04 22:55   ` Victor Corral
2018-04-04 23:57     ` Mark A. Engel
2018-04-05  5:39       ` Mark Geisert
2018-04-05  6:49 ` winpty injection David Macek
2018-04-05  8:47 ` Corinna Vinschen
2018-04-05 13:25   ` Johannes Schindelin
2018-04-05 18:33     ` Thomas Wolff
2018-04-05 19:41       ` Johannes Schindelin
2018-04-05 22:07         ` Thomas Wolff
2018-04-06 10:31           ` Johannes Schindelin
2018-04-06 18:44             ` Thomas Wolff
2018-04-06 19:22               ` Johannes Schindelin
2018-04-09  9:57                 ` Corinna Vinschen
2018-08-16  7:41                   ` Thomas Wolff
2018-08-16  8:54                     ` Johannes Schindelin
2018-08-16 10:36                       ` Thomas Wolff

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).