[PATCH 0/4] Some fixes for PTY with pseudo console support (1)

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/4] Some fixes for PTY with pseudo console support (1)

Takashi Yano
[PATCH 1/4] Cygwin: pty: Code cleanup
- Cleanup the code which is commented out by #if 0 regarding pseudo
  console.
- Remove #if 1 for experimental code which seems to be stable.

[PATCH 2/4] Cygwin: pty: Speed up a little hooked Win32 API for pseudo console.
- Some Win32 APIs are hooked in pty code for pseudo console support.
  This causes slow down. This patch improves speed a little.

[PATCH 3/4] Cygwin: pty: Move function hook_api() into hookapi.cc.
- PTY uses Win32 API hook for pseudo console suppot. The function
  hook_api() is used for this purpose and defined in fhandler_tty.cc
  previously. This patch moves it into hookapi.cc.

[PATCH 4/4] Cygwin: pty: Limit API hook to the program linked with the APIs.
- API hook used for pseudo console support causes slow down.
  This patch limits API hook to only program which is linked
  with the corresponding APIs. Normal cygwin program is not
  linked with such APIs (such as WriteFile, etc...) directly,
  therefore, no slow down occurs. However, console access by
  cygwin.dll itself cannot switch the r/w pipe to pseudo console
  side. Therefore, the code to switch it forcely to pseudo
  console side is added to smallprint.cc and strace.cc.

Takashi Yano (4):
  Cygwin: pty: Code cleanup
  Cygwin: pty: Speed up a little hooked Win32 API for pseudo console.
  Cygwin: pty: Move function hook_api() into hookapi.cc.
  Cygwin: pty: Limit API hook to the program linked with the APIs.

 winsup/cygwin/fhandler_tty.cc | 136 +++++++++++-----------------------
 winsup/cygwin/hookapi.cc      |  34 +++++++++
 winsup/cygwin/smallprint.cc   |   5 ++
 winsup/cygwin/strace.cc       |  29 ++------
 winsup/cygwin/winsup.h        |   1 +
 5 files changed, 89 insertions(+), 116 deletions(-)

--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/4] Cygwin: pty: Code cleanup

Takashi Yano
- Cleanup the code which is commented out by #if 0 regarding pseudo
  console.
- Remove #if 1 for experimental code which seems to be stable.
---
 winsup/cygwin/fhandler_tty.cc | 28 ----------------------------
 1 file changed, 28 deletions(-)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index dd5ab528a..4dbe96b4a 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -855,26 +855,6 @@ fhandler_pty_slave::cleanup ()
 int
 fhandler_pty_slave::close ()
 {
-#if 0
-  if (getPseudoConsole ())
-    {
-      INPUT_RECORD inp[128];
-      DWORD n;
-      PeekFunc =
- PeekConsoleInputA_Orig ? PeekConsoleInputA_Orig : PeekConsoleInput;
-      PeekFunc (get_handle (), inp, 128, &n);
-      bool pipe_empty = true;
-      while (n-- > 0)
- if (inp[n].EventType == KEY_EVENT && inp[n].Event.KeyEvent.bKeyDown)
-  pipe_empty = false;
-      if (pipe_empty)
- {
-  /* Flush input buffer */
-  size_t len = UINT_MAX;
-  read (NULL, len);
- }
-    }
-#endif
   termios_printf ("closing last open %s handle", ttyname ());
   if (inuse && !CloseHandle (inuse))
     termios_printf ("CloseHandle (inuse), %E");
@@ -1524,7 +1504,6 @@ fhandler_pty_slave::read (void *ptr, size_t& len)
 out:
   termios_printf ("%d = read(%p, %lu)", totalread, ptr, len);
   len = (size_t) totalread;
-#if 1 /* Experimenta code */
   /* Push slave read as echo to pseudo console screen buffer. */
   if (getPseudoConsole () && ptr0 && (get_ttyp ()->ti.c_lflag & ECHO))
     {
@@ -1532,7 +1511,6 @@ out:
       push_to_pcon_screenbuffer (ptr0, len);
       release_output_mutex ();
     }
-#endif
   mask_switch_to_pcon (false);
 }
 
@@ -2748,10 +2726,6 @@ restart:
   if (p)
     *p = L'-';
   LCID lcid = LocaleNameToLCID (lc, 0);
-#if 0
-  if (lcid == (LCID) -1)
-    return lcid;
-#endif
   if (!lcid && !strcmp (charset, "ASCII"))
     return 0;
 
@@ -2842,7 +2816,6 @@ fhandler_pty_slave::fixup_after_attach (bool native_maybe)
  }
  }
 
-#if 1 /* Experimental code */
       /* Clear screen to synchronize pseudo console screen buffer
  with real terminal. This is necessary because pseudo
  console screen buffer is empty at start. */
@@ -2854,7 +2827,6 @@ fhandler_pty_slave::fixup_after_attach (bool native_maybe)
  /* Assume this is the first process using this pty slave. */
  WriteFile (get_output_handle_cyg (),
    "\033[H\033[J", 6, &n, NULL);
-#endif
 
       pcon_attached[get_minor ()] = true;
       get_ttyp ()->num_pcon_attached_slaves ++;
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/4] Cygwin: pty: Speed up a little hooked Win32 API for pseudo console.

Takashi Yano
In reply to this post by Takashi Yano
- Some Win32 APIs are hooked in pty code for pseudo console support.
  This causes slow down. This patch improves speed a little.
---
 winsup/cygwin/fhandler_tty.cc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 4dbe96b4a..94ef2f8d4 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -155,7 +155,9 @@ DEF_HOOK (PeekConsoleInputW);
 #define CHK_CONSOLE_ACCESS(h) \
 { \
   DWORD dummy; \
-  if (!isHybrid && GetConsoleMode (h, &dummy)) \
+  if (!isHybrid \
+      && GetFileType (h) == FILE_TYPE_CHAR \
+      && GetConsoleMode (h, &dummy)) \
     { \
       isHybrid = true; \
       set_switch_to_pcon (); \
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 3/4] Cygwin: pty: Move function hook_api() into hookapi.cc.

Takashi Yano
In reply to this post by Takashi Yano
- PTY uses Win32 API hook for pseudo console suppot. The function
  hook_api() is used for this purpose and defined in fhandler_tty.cc
  previously. This patch moves it into hookapi.cc.
---
 winsup/cygwin/fhandler_tty.cc | 44 -----------------------------------
 winsup/cygwin/hookapi.cc      | 34 +++++++++++++++++++++++++++
 winsup/cygwin/winsup.h        |  1 +
 3 files changed, 35 insertions(+), 44 deletions(-)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 94ef2f8d4..f76f7b262 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -75,50 +75,6 @@ static bool pcon_attached[NTTYS];
 static bool isHybrid;
 
 #if USE_API_HOOK
-/* Hook WIN32 API */
-static
-void *hook_api (const char *mname, const char *name, const void *fn)
-{
-  HMODULE hm = GetModuleHandle (mname);
-  PIMAGE_NT_HEADERS pExeNTHdr = PIMAGE_NT_HEADERS (PBYTE (hm)
-   + PIMAGE_DOS_HEADER (hm)->e_lfanew);
-  DWORD importRVA = pExeNTHdr->OptionalHeader.DataDirectory
-    [IMAGE_DIRECTORY_ENTRY_IMPORT].VirtualAddress;
-  PIMAGE_IMPORT_DESCRIPTOR pdfirst =
-    (PIMAGE_IMPORT_DESCRIPTOR) ((char *) hm + importRVA);
-  for (PIMAGE_IMPORT_DESCRIPTOR pd = pdfirst; pd->FirstThunk; pd++)
-    {
-      if (pd->OriginalFirstThunk == 0)
- continue;
-      PIMAGE_THUNK_DATA pt =
- (PIMAGE_THUNK_DATA) ((char *) hm + pd->FirstThunk);
-      PIMAGE_THUNK_DATA pn =
- (PIMAGE_THUNK_DATA) ((char *) hm + pd->OriginalFirstThunk);
-      for (PIMAGE_THUNK_DATA pi = pt; pn->u1.Ordinal; pi++, pn++)
- {
-  if (IMAGE_SNAP_BY_ORDINAL (pn->u1.Ordinal))
-    continue;
-  PIMAGE_IMPORT_BY_NAME pimp =
-    (PIMAGE_IMPORT_BY_NAME) ((char *) hm + pn->u1.AddressOfData);
-  if (strcmp (name, (char *) pimp->Name) != 0)
-    continue;
-#ifdef __x86_64__
-#define THUNK_FUNC_TYPE ULONGLONG
-#else
-#define THUNK_FUNC_TYPE DWORD
-#endif
-  DWORD ofl = PAGE_READWRITE;
-  if (!VirtualProtect (pi, sizeof (THUNK_FUNC_TYPE), ofl, &ofl))
-    return NULL;
-  void *origfn = (void *) pi->u1.Function;
-  pi->u1.Function = (THUNK_FUNC_TYPE) fn;
-  VirtualProtect (pi, sizeof (THUNK_FUNC_TYPE), ofl, &ofl);
-  return origfn;
- }
-    }
-  return NULL;
-}
-
 static void
 set_switch_to_pcon (void)
 {
diff --git a/winsup/cygwin/hookapi.cc b/winsup/cygwin/hookapi.cc
index 4078e65bd..dcd9b1df8 100644
--- a/winsup/cygwin/hookapi.cc
+++ b/winsup/cygwin/hookapi.cc
@@ -428,6 +428,40 @@ hook_or_detect_cygwin (const char *name, const void *fn, WORD& subsys, HANDLE h)
   return fh.origfn;
 }
 
+/* Hook a function in any DLL such as kernel32.dll */
+/* The DLL must be loaded in advance. */
+/* Used in fhandler_tty.cc */
+void *hook_api (const char *mname, const char *name, const void *fn)
+{
+  HMODULE hm = GetModuleHandle (mname);
+  PIMAGE_NT_HEADERS pExeNTHdr =
+    rva (PIMAGE_NT_HEADERS, hm, PIMAGE_DOS_HEADER (hm)->e_lfanew);
+  DWORD importRVA = pExeNTHdr->OptionalHeader.DataDirectory
+    [IMAGE_DIRECTORY_ENTRY_IMPORT].VirtualAddress;
+  PIMAGE_IMPORT_DESCRIPTOR pdfirst =
+    rva (PIMAGE_IMPORT_DESCRIPTOR, hm, importRVA);
+  for (PIMAGE_IMPORT_DESCRIPTOR pd = pdfirst; pd->FirstThunk; pd++)
+    {
+      if (pd->OriginalFirstThunk == 0)
+ continue;
+      PIMAGE_THUNK_DATA pt = rva (PIMAGE_THUNK_DATA, hm, pd->FirstThunk);
+      PIMAGE_THUNK_DATA pn =
+ rva (PIMAGE_THUNK_DATA, hm, pd->OriginalFirstThunk);
+      for (PIMAGE_THUNK_DATA pi = pt; pn->u1.Ordinal; pi++, pn++)
+ {
+  if (IMAGE_SNAP_BY_ORDINAL (pn->u1.Ordinal))
+    continue;
+  PIMAGE_IMPORT_BY_NAME pimp =
+    rva (PIMAGE_IMPORT_BY_NAME, hm, pn->u1.AddressOfData);
+  if (strcmp (name, (char *) pimp->Name) != 0)
+    continue;
+  void *origfn = putmem (pi, fn);
+  return origfn;
+ }
+    }
+  return NULL;
+}
+
 void
 ld_preload ()
 {
diff --git a/winsup/cygwin/winsup.h b/winsup/cygwin/winsup.h
index 95ab41e6b..ab7b3bbdc 100644
--- a/winsup/cygwin/winsup.h
+++ b/winsup/cygwin/winsup.h
@@ -199,6 +199,7 @@ ino_t __reg2 hash_path_name (ino_t hash, const char *name);
 void __reg2 nofinalslash (const char *src, char *dst);
 
 void __reg3 *hook_or_detect_cygwin (const char *, const void *, WORD&, HANDLE h = NULL);
+void __reg3 *hook_api (const char *mname, const char *name, const void *fn);
 
 /* Time related */
 void __stdcall totimeval (struct timeval *, PLARGE_INTEGER, int, int);
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 4/4] Cygwin: pty: Limit API hook to the program linked with the APIs.

Takashi Yano
In reply to this post by Takashi Yano
- API hook used for pseudo console support causes slow down.
  This patch limits API hook to only program which is linked
  with the corresponding APIs. Normal cygwin program is not
  linked with such APIs (such as WriteFile, etc...) directly,
  therefore, no slow down occurs. However, console access by
  cygwin.dll itself cannot switch the r/w pipe to pseudo console
  side. Therefore, the code to switch it forcely to pseudo
  console side is added to smallprint.cc and strace.cc.
---
 winsup/cygwin/fhandler_tty.cc | 60 ++++++++++++++++++++++++-----------
 winsup/cygwin/smallprint.cc   |  5 +++
 winsup/cygwin/strace.cc       | 29 +++--------------
 3 files changed, 51 insertions(+), 43 deletions(-)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index f76f7b262..3a23083de 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -89,6 +89,18 @@ set_switch_to_pcon (void)
       }
 }
 
+void set_ishybrid_and_switch_to_pcon (HANDLE h)
+{
+  DWORD dummy;
+  if (!isHybrid
+      && GetFileType (h) == FILE_TYPE_CHAR
+      && GetConsoleMode (h, &dummy))
+    {
+      isHybrid = true;
+      set_switch_to_pcon ();
+    }
+}
+
 #define DEF_HOOK(name) static __typeof__ (name) *name##_Orig
 DEF_HOOK (WriteFile);
 DEF_HOOK (WriteConsoleA);
@@ -101,6 +113,7 @@ DEF_HOOK (WriteConsoleOutputW);
 DEF_HOOK (WriteConsoleOutputCharacterA);
 DEF_HOOK (WriteConsoleOutputCharacterW);
 DEF_HOOK (WriteConsoleOutputAttribute);
+DEF_HOOK (SetConsoleTextAttribute);
 DEF_HOOK (WriteConsoleInputA);
 DEF_HOOK (WriteConsoleInputW);
 DEF_HOOK (ReadConsoleInputA);
@@ -197,6 +210,13 @@ WriteConsoleOutputAttribute_Hooked
   return WriteConsoleOutputAttribute_Orig (h, a, l, c, n);
 }
 static BOOL WINAPI
+SetConsoleTextAttribute_Hooked
+     (HANDLE h, WORD a)
+{
+  CHK_CONSOLE_ACCESS (h);
+  return SetConsoleTextAttribute_Orig (h, a);
+}
+static BOOL WINAPI
 WriteConsoleInputA_Hooked
      (HANDLE h, CONST INPUT_RECORD *r, DWORD l, LPDWORD n)
 {
@@ -242,6 +262,7 @@ PeekConsoleInputW_Hooked
 #define WriteFile_Orig 0
 #define ReadFile_Orig 0
 #define PeekConsoleInputA_Orig 0
+void set_ishybrid_and_switch_to_pcon (void) {}
 #endif /* USE_API_HOOK */
 
 bool
@@ -2855,25 +2876,26 @@ fhandler_pty_slave::fixup_after_exec ()
  { \
   void *api = hook_api (module, #name, (void *) name##_Hooked); \
   name##_Orig = (__typeof__ (name) *) api; \
-  if (!api) system_printf("Hooking " #name " failed."); \
- }
-      DO_HOOK ("kernel32.dll", WriteFile);
-      DO_HOOK ("kernel32.dll", WriteConsoleA);
-      DO_HOOK ("kernel32.dll", WriteConsoleW);
-      DO_HOOK ("kernel32.dll", ReadFile);
-      DO_HOOK ("kernel32.dll", ReadConsoleA);
-      DO_HOOK ("kernel32.dll", ReadConsoleW);
-      DO_HOOK ("kernel32.dll", WriteConsoleOutputA);
-      DO_HOOK ("kernel32.dll", WriteConsoleOutputW);
-      DO_HOOK ("kernel32.dll", WriteConsoleOutputCharacterA);
-      DO_HOOK ("kernel32.dll", WriteConsoleOutputCharacterW);
-      DO_HOOK ("kernel32.dll", WriteConsoleOutputAttribute);
-      DO_HOOK ("kernel32.dll", WriteConsoleInputA);
-      DO_HOOK ("kernel32.dll", WriteConsoleInputW);
-      DO_HOOK ("kernel32.dll", ReadConsoleInputA);
-      DO_HOOK ("kernel32.dll", ReadConsoleInputW);
-      DO_HOOK ("kernel32.dll", PeekConsoleInputA);
-      DO_HOOK ("kernel32.dll", PeekConsoleInputW);
+  /*if (api) system_printf(#name " hooked.");*/ \
+ }
+      DO_HOOK (NULL, WriteFile);
+      DO_HOOK (NULL, WriteConsoleA);
+      DO_HOOK (NULL, WriteConsoleW);
+      DO_HOOK (NULL, ReadFile);
+      DO_HOOK (NULL, ReadConsoleA);
+      DO_HOOK (NULL, ReadConsoleW);
+      DO_HOOK (NULL, WriteConsoleOutputA);
+      DO_HOOK (NULL, WriteConsoleOutputW);
+      DO_HOOK (NULL, WriteConsoleOutputCharacterA);
+      DO_HOOK (NULL, WriteConsoleOutputCharacterW);
+      DO_HOOK (NULL, WriteConsoleOutputAttribute);
+      DO_HOOK (NULL, SetConsoleTextAttribute);
+      DO_HOOK (NULL, WriteConsoleInputA);
+      DO_HOOK (NULL, WriteConsoleInputW);
+      DO_HOOK (NULL, ReadConsoleInputA);
+      DO_HOOK (NULL, ReadConsoleInputW);
+      DO_HOOK (NULL, PeekConsoleInputA);
+      DO_HOOK (NULL, PeekConsoleInputW);
     }
 #endif /* USE_API_HOOK */
 }
diff --git a/winsup/cygwin/smallprint.cc b/winsup/cygwin/smallprint.cc
index a7a19132b..bd19c1f5f 100644
--- a/winsup/cygwin/smallprint.cc
+++ b/winsup/cygwin/smallprint.cc
@@ -384,6 +384,9 @@ __small_sprintf (char *dst, const char *fmt, ...)
   return r;
 }
 
+/* Defined in fhandler_tty.cc */
+extern void set_ishybrid_and_switch_to_pcon (HANDLE);
+
 void
 small_printf (const char *fmt, ...)
 {
@@ -405,6 +408,7 @@ small_printf (const char *fmt, ...)
   count = __small_vsprintf (buf, fmt, ap);
   va_end (ap);
 
+  set_ishybrid_and_switch_to_pcon (GetStdHandle (STD_ERROR_HANDLE));
   WriteFile (GetStdHandle (STD_ERROR_HANDLE), buf, count, &done, NULL);
   FlushFileBuffers (GetStdHandle (STD_ERROR_HANDLE));
 }
@@ -431,6 +435,7 @@ console_printf (const char *fmt, ...)
   count = __small_vsprintf (buf, fmt, ap);
   va_end (ap);
 
+  set_ishybrid_and_switch_to_pcon (console_handle);
   WriteFile (console_handle, buf, count, &done, NULL);
   FlushFileBuffers (console_handle);
 }
diff --git a/winsup/cygwin/strace.cc b/winsup/cygwin/strace.cc
index b1eb5f3e4..fc3d0b783 100644
--- a/winsup/cygwin/strace.cc
+++ b/winsup/cygwin/strace.cc
@@ -243,6 +243,9 @@ strace::write_childpid (pid_t pid)
 static NO_COPY muto strace_buf_guard;
 static NO_COPY char *buf;
 
+/* Defined in fhandler_tty.cc */
+extern void set_ishybrid_and_switch_to_pcon (HANDLE);
+
 void
 strace::vprntf (unsigned category, const char *func, const char *fmt, va_list ap)
 {
@@ -264,6 +267,7 @@ strace::vprntf (unsigned category, const char *func, const char *fmt, va_list ap
   if (category & _STRACE_SYSTEM)
     {
       DWORD done;
+      set_ishybrid_and_switch_to_pcon (GetStdHandle (STD_ERROR_HANDLE));
       WriteFile (GetStdHandle (STD_ERROR_HANDLE), buf, len, &done, 0);
       FlushFileBuffers (GetStdHandle (STD_ERROR_HANDLE));
       /* Make sure that the message shows up on the screen, too, since this is
@@ -275,34 +279,11 @@ strace::vprntf (unsigned category, const char *func, const char *fmt, va_list ap
  &sec_none, OPEN_EXISTING, 0, 0);
   if (h != INVALID_HANDLE_VALUE)
     {
+      set_ishybrid_and_switch_to_pcon (h);
       WriteFile (h, buf, len, &done, 0);
       CloseHandle (h);
     }
  }
-#if 1 /* Experimental code */
-      /* PTY with pseudo console cannot display data written to
- STD_ERROR_HANDLE (output_handle) if the process is cygwin
- process. output_handle works only in native console apps.
- Therefore the data should be written to output_handle_cyg
- as well. */
-      fhandler_base *fh = ::cygheap->fdtab[2];
-      if (fh && fh->get_major () == DEV_PTYS_MAJOR)
- {
-  fhandler_pty_slave *ptys = (fhandler_pty_slave *) fh;
-  if (ptys->getPseudoConsole ())
-    {
-      HANDLE h_cyg = ptys->get_output_handle_cyg ();
-      if (buf[len-1] == '\n' && len < NT_MAX_PATH - 1)
- {
-  buf[len-1] = '\r';
-  buf[len] = '\n';
-  len ++;
- }
-      WriteFile (h_cyg, buf, len, &done, 0);
-      FlushFileBuffers (h_cyg);
-    }
- }
-#endif
     }
 
 #ifndef NOSTRACE
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/4] Cygwin: pty: Limit API hook to the program linked with the APIs.

Corinna Vinschen-2
Hi Takashi,

On Sep  4 10:44, Takashi Yano wrote:
> - API hook used for pseudo console support causes slow down.
>   This patch limits API hook to only program which is linked
>   with the corresponding APIs. Normal cygwin program is not
>   linked with such APIs (such as WriteFile, etc...) directly,
>   therefore, no slow down occurs. However, console access by
>   cygwin.dll itself cannot switch the r/w pipe to pseudo console
>   side. Therefore, the code to switch it forcely to pseudo
>   console side is added to smallprint.cc and strace.cc.

I'll push the other 3 patches from this series.  For this patch,
I wonder why you create set_ishybrid_and_switch_to_pcon while
at the same time define a macro CHK_CONSOLE_ACCESS with identical
functionality.

Suggestion: Only define set_ishybrid_and_switch_to_pcon() as
inline function (probably in winsup.h) and use only this througout.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/4] Cygwin: pty: Limit API hook to the program linked with the APIs.

Takashi Yano
Hi Corinna,

On Wed, 4 Sep 2019 12:03:51 +0200
Corinna Vinschen wrote:
> I'll push the other 3 patches from this series.  For this patch,
> I wonder why you create set_ishybrid_and_switch_to_pcon while
> at the same time define a macro CHK_CONSOLE_ACCESS with identical
> functionality.

Yah, indeed!

> Suggestion: Only define set_ishybrid_and_switch_to_pcon() as
> inline function (probably in winsup.h) and use only this througout.

This function uses static variable isHybrid (sorry camelback again)
and static function set_switch_to_pcon() defined in fhandler_tty.cc.

To make it inline, a lot of changes will be necessary. How about
non-inline function?

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/4] Cygwin: pty: Limit API hook to the program linked with the APIs.

Corinna Vinschen-2
Hi Takashi,

On Sep  4 21:39, Takashi Yano wrote:

> Hi Corinna,
>
> On Wed, 4 Sep 2019 12:03:51 +0200
> Corinna Vinschen wrote:
> > I'll push the other 3 patches from this series.  For this patch,
> > I wonder why you create set_ishybrid_and_switch_to_pcon while
> > at the same time define a macro CHK_CONSOLE_ACCESS with identical
> > functionality.
>
> Yah, indeed!
>
> > Suggestion: Only define set_ishybrid_and_switch_to_pcon() as
> > inline function (probably in winsup.h) and use only this througout.
>
> This function uses static variable isHybrid (sorry camelback again)
> and static function set_switch_to_pcon() defined in fhandler_tty.cc.
>
> To make it inline, a lot of changes will be necessary. How about
> non-inline function?
That will add extra function calls, but, yeah, sure.  We can streamline
this later.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment