[PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Takashi Yano]

Takashi Yano (2):
  Cygwin: syscalls.cc: Make _get_osfhandle() return appropriate handle.
  Cygwin: pty: Add hook for GetStdHandle() to return appropriate handle.

 winsup/cygwin/fhandler_tty.cc | 19 +++++++++++++++++++
 winsup/cygwin/syscalls.cc     | 13 ++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

-- 
2.30.1

[PATCH 1/2] Cygwin: syscalls.cc: Make _get_osfhandle() return appropriate handle.

[Takashi Yano]

- Currently, _get_osfhandle() returns input handle for pty even for
  stdout and stdout. This patch fixes the issue. Also, setup_locale()
  is called to make sure the charset conversion works for output.
---
 winsup/cygwin/syscalls.cc | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 6ba4f10f7..205d15951 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -3223,7 +3223,18 @@ _get_osfhandle (int fd)
 
   cygheap_fdget cfd (fd);
   if (cfd >= 0)
-    res = (long) cfd->get_handle ();
+    {
+      if (cfd->get_major () == DEV_PTYS_MAJOR)
+	{
+	  fhandler_pty_slave *ptys =
+	    (fhandler_pty_slave *) (fhandler_base *) cfd;
+	  ptys->setup_locale ();
+	}
+      if (fd == 1 || fd == 2)
+	res = (long) cfd->get_output_handle ();
+      else
+	res = (long) cfd->get_handle_cyg ();
+    }
   else
     res = -1;
 
-- 
2.30.1

[PATCH 2/2] Cygwin: pty: Add hook for GetStdHandle() to return appropriate handle.

[Takashi Yano]

- Currently, GetStdHandle(STD_INPUT_HANDLE) returns input handle for
  non-cygwin process. If cygwin process read from non-cygwin pipe,
  this causes hang up because master writes input to cygwin pipe.
  Also, setup_locale() is called to make charset conversion for output
  handle work properly.
---
 winsup/cygwin/fhandler_tty.cc | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 02e94efcc..c1f11f399 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -157,6 +157,7 @@ set_switch_to_pcon (HANDLE *in, HANDLE *out, HANDLE *err, bool iscygwin)
 DEF_HOOK (CreateProcessA);
 DEF_HOOK (CreateProcessW);
 DEF_HOOK (exit);
+DEF_HOOK (GetStdHandle);
 
 static BOOL WINAPI
 CreateProcessA_Hooked
@@ -300,6 +301,23 @@ exit_Hooked (int e)
   exit_Orig (e);
 }
 
+static HANDLE WINAPI
+GetStdHandle_Hooked (DWORD h)
+{
+  HANDLE r = GetStdHandle_Orig (h);
+  cygheap_fdenum cfd (false);
+  while (cfd.next () >= 0)
+    if (cfd->get_major () == DEV_PTYS_MAJOR)
+      {
+	fhandler_pty_slave *ptys =
+	  (fhandler_pty_slave *) (fhandler_base *) cfd;
+	ptys->setup_locale ();
+	if (r == cfd->get_handle ())
+	  return cfd->get_handle_cyg ();
+      }
+  return r;
+}
+
 static void
 convert_mb_str (UINT cp_to, char *ptr_to, size_t *len_to,
 		UINT cp_from, const char *ptr_from, size_t len_from,
@@ -2349,6 +2367,7 @@ fhandler_pty_slave::fixup_after_exec ()
   DO_HOOK (NULL, CreateProcessW);
   if (CreateProcessA_Orig || CreateProcessW_Orig)
     DO_HOOK (NULL, exit);
+  DO_HOOK (NULL, GetStdHandle);
 }
 
 /* This thread function handles the master control pipe.  It waits for a
-- 
2.30.1

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Takashi Yano]

On Sun, 21 Mar 2021 13:01:24 +0900
Takashi Yano wrote:
> Takashi Yano (2):
>   Cygwin: syscalls.cc: Make _get_osfhandle() return appropriate handle.
>   Cygwin: pty: Add hook for GetStdHandle() to return appropriate handle.
> 
>  winsup/cygwin/fhandler_tty.cc | 19 +++++++++++++++++++
>  winsup/cygwin/syscalls.cc     | 13 ++++++++++++-
>  2 files changed, 31 insertions(+), 1 deletion(-)

I submitted these patches, however, I still wonder if we really
need these patches. I cannot imagine the situation where handle
itself is needed rather than file descriptor.

However, following cygwin apps/dlls call _get_osfhandle():
ccmake.exe
cmake.exe
cpack.exe
ctest.exe
ddrescue.exe

And also, following cygwin apps/dlls call GetStdHandle():
ccmake.exe
cmake.exe
cpack.exe
ctest.exe
run.exe
cygusb0.dll
tk86.dll

in my installation.

Therefore, some of these apps/dlls may need these patches...

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Takashi Yano]

On Sun, 21 Mar 2021 17:44:27 +0900
Takashi Yano wrote:
> On Sun, 21 Mar 2021 13:01:24 +0900
> Takashi Yano wrote:
> > Takashi Yano (2):
> >   Cygwin: syscalls.cc: Make _get_osfhandle() return appropriate handle.
> >   Cygwin: pty: Add hook for GetStdHandle() to return appropriate handle.
> > 
> >  winsup/cygwin/fhandler_tty.cc | 19 +++++++++++++++++++
> >  winsup/cygwin/syscalls.cc     | 13 ++++++++++++-
> >  2 files changed, 31 insertions(+), 1 deletion(-)
> 
> I submitted these patches, however, I still wonder if we really
> need these patches. I cannot imagine the situation where handle
> itself is needed rather than file descriptor.
> 
> However, following cygwin apps/dlls call _get_osfhandle():
> ccmake.exe
> cmake.exe
> cpack.exe
> ctest.exe
> ddrescue.exe
> 
> And also, following cygwin apps/dlls call GetStdHandle():
> ccmake.exe
> cmake.exe
> cpack.exe
> ctest.exe
> run.exe
> cygusb0.dll
> tk86.dll
> 
> in my installation.
> 
> Therefore, some of these apps/dlls may need these patches...

I looked into cmake source and found the patch exactly for
this issue. Therefore, it seems better to fix this. 

/* Get the Windows handle for a FILE stream.  */
static HANDLE kwsysTerminalGetStreamHandle(FILE* stream)
{
  /* Get the C-library file descriptor from the stream.  */
  int fd = fileno(stream);

#  if defined(__CYGWIN__)
  /* Cygwin seems to have an extra pipe level.  If the file descriptor
     corresponds to stdout or stderr then obtain the matching windows
     handle directly.  */
  if (fd == fileno(stdout)) {
    return GetStdHandle(STD_OUTPUT_HANDLE);
  } else if (fd == fileno(stderr)) {
    return GetStdHandle(STD_ERROR_HANDLE);
  }
#  endif

  /* Get the underlying Windows handle for the descriptor.  */
  return (HANDLE)_get_osfhandle(fd);
}


-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Corinna Vinschen]

[CC Marco]

On Mar 22 08:07, Takashi Yano via Cygwin-patches wrote:
> On Sun, 21 Mar 2021 17:44:27 +0900
> Takashi Yano wrote:
> > On Sun, 21 Mar 2021 13:01:24 +0900
> > Takashi Yano wrote:
> > > Takashi Yano (2):
> > >   Cygwin: syscalls.cc: Make _get_osfhandle() return appropriate handle.
> > >   Cygwin: pty: Add hook for GetStdHandle() to return appropriate handle.
> > > 
> > >  winsup/cygwin/fhandler_tty.cc | 19 +++++++++++++++++++
> > >  winsup/cygwin/syscalls.cc     | 13 ++++++++++++-
> > >  2 files changed, 31 insertions(+), 1 deletion(-)
> > 
> > I submitted these patches, however, I still wonder if we really
> > need these patches. I cannot imagine the situation where handle
> > itself is needed rather than file descriptor.
> > 
> > However, following cygwin apps/dlls call _get_osfhandle():
> > ccmake.exe
> > cmake.exe
> > cpack.exe
> > ctest.exe
> > ddrescue.exe
> > 
> > And also, following cygwin apps/dlls call GetStdHandle():
> > ccmake.exe
> > cmake.exe
> > cpack.exe
> > ctest.exe
> > run.exe
> > cygusb0.dll
> > tk86.dll
> > 
> > in my installation.
> > 
> > Therefore, some of these apps/dlls may need these patches...
> 
> I looked into cmake source and found the patch exactly for
> this issue. Therefore, it seems better to fix this. 
> 
> /* Get the Windows handle for a FILE stream.  */
> static HANDLE kwsysTerminalGetStreamHandle(FILE* stream)
> {
>   /* Get the C-library file descriptor from the stream.  */
>   int fd = fileno(stream);
> 
> #  if defined(__CYGWIN__)
>   /* Cygwin seems to have an extra pipe level.  If the file descriptor
>      corresponds to stdout or stderr then obtain the matching windows
>      handle directly.  */
>   if (fd == fileno(stdout)) {
>     return GetStdHandle(STD_OUTPUT_HANDLE);
>   } else if (fd == fileno(stderr)) {
>     return GetStdHandle(STD_ERROR_HANDLE);
>   }
> #  endif
> 
>   /* Get the underlying Windows handle for the descriptor.  */
>   return (HANDLE)_get_osfhandle(fd);
> }

Why on earth is cmake using Windows functions on Cygwin at all???
It's not as if it actually requires Windows functionality on our
platform.

Marco, any input?  Any chance to drop this Windows stuff from the Cygwin
code path in cmake?


Thanks,
Corinna

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Ken Brown]

[Still CC Marco]

On 3/22/2021 7:43 AM, Corinna Vinschen via Cygwin-patches wrote:
> [CC Marco]
> 
> On Mar 22 08:07, Takashi Yano via Cygwin-patches wrote:
>> On Sun, 21 Mar 2021 17:44:27 +0900
>> Takashi Yano wrote:
>>> On Sun, 21 Mar 2021 13:01:24 +0900
>>> Takashi Yano wrote:
>>>> Takashi Yano (2):
>>>>    Cygwin: syscalls.cc: Make _get_osfhandle() return appropriate handle.
>>>>    Cygwin: pty: Add hook for GetStdHandle() to return appropriate handle.
>>>>
>>>>   winsup/cygwin/fhandler_tty.cc | 19 +++++++++++++++++++
>>>>   winsup/cygwin/syscalls.cc     | 13 ++++++++++++-
>>>>   2 files changed, 31 insertions(+), 1 deletion(-)
>>>
>>> I submitted these patches, however, I still wonder if we really
>>> need these patches. I cannot imagine the situation where handle
>>> itself is needed rather than file descriptor.
>>>
>>> However, following cygwin apps/dlls call _get_osfhandle():
>>> ccmake.exe
>>> cmake.exe
>>> cpack.exe
>>> ctest.exe
>>> ddrescue.exe
>>>
>>> And also, following cygwin apps/dlls call GetStdHandle():
>>> ccmake.exe
>>> cmake.exe
>>> cpack.exe
>>> ctest.exe
>>> run.exe
>>> cygusb0.dll
>>> tk86.dll
>>>
>>> in my installation.
>>>
>>> Therefore, some of these apps/dlls may need these patches...
>>
>> I looked into cmake source and found the patch exactly for
>> this issue. Therefore, it seems better to fix this.
>>
>> /* Get the Windows handle for a FILE stream.  */
>> static HANDLE kwsysTerminalGetStreamHandle(FILE* stream)
>> {
>>    /* Get the C-library file descriptor from the stream.  */
>>    int fd = fileno(stream);
>>
>> #  if defined(__CYGWIN__)
>>    /* Cygwin seems to have an extra pipe level.  If the file descriptor
>>       corresponds to stdout or stderr then obtain the matching windows
>>       handle directly.  */
>>    if (fd == fileno(stdout)) {
>>      return GetStdHandle(STD_OUTPUT_HANDLE);
>>    } else if (fd == fileno(stderr)) {
>>      return GetStdHandle(STD_ERROR_HANDLE);
>>    }
>> #  endif
>>
>>    /* Get the underlying Windows handle for the descriptor.  */
>>    return (HANDLE)_get_osfhandle(fd);
>> }
> 
> Why on earth is cmake using Windows functions on Cygwin at all???
> It's not as if it actually requires Windows functionality on our
> platform.

Out of curiosity, I took a quick glance at the cmake code.  It appears that this 
code is designed to support running cmake in a Console.  I don't think that 
should be needed any more, if it ever was.

> Marco, any input?  Any chance to drop this Windows stuff from the Cygwin
> code path in cmake?

I think the following might suffice (untested):

--- a/Source/kwsys/Terminal.c
+++ b/Source/kwsys/Terminal.c
@@ -10,7 +10,7 @@
  #endif

  /* Configure support for this platform.  */
-#if defined(_WIN32) || defined(__CYGWIN__)
+#if defined(_WIN32)
  #  define KWSYS_TERMINAL_SUPPORT_CONSOLE
  #endif
  #if !defined(_WIN32)

Ken

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Corinna Vinschen]

[CC Marco, CC Jan]

On Mar 22 13:02, Ken Brown via Cygwin-patches wrote:
> [Still CC Marco]
> 
> On 3/22/2021 7:43 AM, Corinna Vinschen via Cygwin-patches wrote:
> > [CC Marco]
> > 
> > On Mar 22 08:07, Takashi Yano via Cygwin-patches wrote:
> > > On Sun, 21 Mar 2021 17:44:27 +0900
> > > Takashi Yano wrote:
> > > > [...]
> > > > However, following cygwin apps/dlls call _get_osfhandle():
> > > > ccmake.exe
> > > > cmake.exe
> > > > cpack.exe
> > > > ctest.exe
> > > > ddrescue.exe

I'm pretty sure ddrescue needs the osfhandle just to access raw block
devices.

> > > > And also, following cygwin apps/dlls call GetStdHandle():
> > > > ccmake.exe
> > > > cmake.exe
> > > > cpack.exe
> > > > ctest.exe
> > > > run.exe

run creates its own conin/conout handles to create a hidden console.
The code calling GetStdHandle() is only for debug purposes and never
built into the executable.

> > > > cygusb0.dll

This lib tries to access USB devices only.

> > > > tk86.dll

Not sure about this one.  In theory this shouldn't happen, given our
tk is built against X11, not against the Windows GUI.

Jan, can you please check where and why tk86.dll calls GetStdHandle.
I found a few places in the source where GetStdHandle is called, but
it's not clear to me which one is called.

> Out of curiosity, I took a quick glance at the cmake code.  It appears that
> this code is designed to support running cmake in a Console.  I don't think
> that should be needed any more, if it ever was.
> [...]
> I think the following might suffice (untested):
> 
> --- a/Source/kwsys/Terminal.c
> +++ b/Source/kwsys/Terminal.c
> @@ -10,7 +10,7 @@
>  #endif
> 
>  /* Configure support for this platform.  */
> -#if defined(_WIN32) || defined(__CYGWIN__)
> +#if defined(_WIN32)
>  #  define KWSYS_TERMINAL_SUPPORT_CONSOLE
>  #endif
>  #if !defined(_WIN32)

Looks right to me.  If we patch cmake to do the right thing, do we still
need this patch, Takashi?


Thanks,
Corinna

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Takashi Yano]

On Tue, 23 Mar 2021 11:10:04 +0100
Corinna Vinschen wrote:
> [CC Marco, CC Jan]
> 
> On Mar 22 13:02, Ken Brown via Cygwin-patches wrote:
> > [Still CC Marco]
> > 
> > On 3/22/2021 7:43 AM, Corinna Vinschen via Cygwin-patches wrote:
> > > [CC Marco]
> > > 
> > > On Mar 22 08:07, Takashi Yano via Cygwin-patches wrote:
> > > > On Sun, 21 Mar 2021 17:44:27 +0900
> > > > Takashi Yano wrote:
> > > > > [...]
> > > > > However, following cygwin apps/dlls call _get_osfhandle():
> > > > > ccmake.exe
> > > > > cmake.exe
> > > > > cpack.exe
> > > > > ctest.exe
> > > > > ddrescue.exe
> 
> I'm pretty sure ddrescue needs the osfhandle just to access raw block
> devices.
> 
> > > > > And also, following cygwin apps/dlls call GetStdHandle():
> > > > > ccmake.exe
> > > > > cmake.exe
> > > > > cpack.exe
> > > > > ctest.exe
> > > > > run.exe
> 
> run creates its own conin/conout handles to create a hidden console.
> The code calling GetStdHandle() is only for debug purposes and never
> built into the executable.
> 
> > > > > cygusb0.dll
> 
> This lib tries to access USB devices only.
> 
> > > > > tk86.dll
> 
> Not sure about this one.  In theory this shouldn't happen, given our
> tk is built against X11, not against the Windows GUI.
> 
> Jan, can you please check where and why tk86.dll calls GetStdHandle.
> I found a few places in the source where GetStdHandle is called, but
> it's not clear to me which one is called.

Thanks for checking obove.

> > Out of curiosity, I took a quick glance at the cmake code.  It appears that
> > this code is designed to support running cmake in a Console.  I don't think
> > that should be needed any more, if it ever was.
> > [...]
> > I think the following might suffice (untested):
> > 
> > --- a/Source/kwsys/Terminal.c
> > +++ b/Source/kwsys/Terminal.c
> > @@ -10,7 +10,7 @@
> >  #endif
> > 
> >  /* Configure support for this platform.  */
> > -#if defined(_WIN32) || defined(__CYGWIN__)
> > +#if defined(_WIN32)
> >  #  define KWSYS_TERMINAL_SUPPORT_CONSOLE
> >  #endif
> >  #if !defined(_WIN32)
> 
> Looks right to me.  If we patch cmake to do the right thing, do we still
> need this patch, Takashi?

I don't think so. If all is well with current code, nothing to be fixed.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Corinna Vinschen]

On Mar 23 20:57, Takashi Yano via Cygwin-patches wrote:
> Corinna Vinschen wrote:
> > > > On Mar 22 08:07, Takashi Yano via Cygwin-patches wrote:
> > > > > > And also, following cygwin apps/dlls call GetStdHandle():
> > > > > > ccmake.exe
> > > > > > cmake.exe
> > > > > > cpack.exe
> > > > > > ctest.exe
> > > > > > run.exe
> > 
> > run creates its own conin/conout handles to create a hidden console.
> > The code calling GetStdHandle() is only for debug purposes and never
> > built into the executable.

Sorry, but this was utterly wrong.  run calls GetStdHandle, then
overwrites the handles, but only if it doesn't already is attached to a
console.

> > Looks right to me.  If we patch cmake to do the right thing, do we still
> > need this patch, Takashi?
> 
> I don't think so. If all is well with current code, nothing to be fixed.

How do you evaluate this in light of the run behaviour above?


Thanks,
Corinna

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Takashi Yano]

On Tue, 23 Mar 2021 13:17:16 +0100
Corinna Vinschen wrote:
> On Mar 23 20:57, Takashi Yano via Cygwin-patches wrote:
> > Corinna Vinschen wrote:
> > > > > On Mar 22 08:07, Takashi Yano via Cygwin-patches wrote:
> > > > > > > And also, following cygwin apps/dlls call GetStdHandle():
> > > > > > > ccmake.exe
> > > > > > > cmake.exe
> > > > > > > cpack.exe
> > > > > > > ctest.exe
> > > > > > > run.exe
> > > 
> > > run creates its own conin/conout handles to create a hidden console.
> > > The code calling GetStdHandle() is only for debug purposes and never
> > > built into the executable.
> 
> Sorry, but this was utterly wrong.  run calls GetStdHandle, then
> overwrites the handles, but only if it doesn't already is attached to a
> console.
> 
> > > Looks right to me.  If we patch cmake to do the right thing, do we still
> > > need this patch, Takashi?
> > 
> > I don't think so. If all is well with current code, nothing to be fixed.
> 
> How do you evaluate this in light of the run behaviour above?

I try to check run.exe behaviour and noticed that
run cmd.exe
and
run cat.exe
does not work with cygwin 3.0.7 and 3.2.0 (TEST) while these
work in 3.1.7.

Is this expected behaviour?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Takashi Yano]

On Tue, 23 Mar 2021 21:32:12 +0900
Takashi Yano wrote:
> I try to check run.exe behaviour and noticed that
> run cmd.exe
> and
> run cat.exe
> does not work with cygwin 3.0.7 and 3.2.0 (TEST) while these
> work in 3.1.7.

In obove cases, cmd.exe and cat.exe is running in *hidden* console,
therefore nothing is shown. Right?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Corinna Vinschen]

On Mar 23 21:42, Takashi Yano via Cygwin-patches wrote:
> On Tue, 23 Mar 2021 21:32:12 +0900
> Takashi Yano wrote:
> > I try to check run.exe behaviour and noticed that
> > run cmd.exe
> > and
> > run cat.exe
> > does not work with cygwin 3.0.7 and 3.2.0 (TEST) while these
> > work in 3.1.7.
> 
> In obove cases, cmd.exe and cat.exe is running in *hidden* console,
> therefore nothing is shown. Right?

Yes, that's it.  In my other reply I wrote something about running
without console, but in fact that's running without *visible* console,
so, yeah, setting up the hidden console is it's task.


Corinna

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Takashi Yano]

On Tue, 23 Mar 2021 21:42:06 +0900
Takashi Yano wrote:
> On Tue, 23 Mar 2021 21:32:12 +0900
> Takashi Yano wrote:
> > I try to check run.exe behaviour and noticed that
> > run cmd.exe
> > and
> > run cat.exe
> > does not work with cygwin 3.0.7 and 3.2.0 (TEST) while these
> > work in 3.1.7.
> 
> In obove cases, cmd.exe and cat.exe is running in *hidden* console,
> therefore nothing is shown. Right?

In what situation are
  psi->cb = sizeof (STARTUPINFO);
  psi->hStdInput  = GetStdHandle (STD_INPUT_HANDLE);
  psi->hStdOutput = GetStdHandle (STD_OUTPUT_HANDLE);
  psi->hStdError  = GetStdHandle (STD_ERROR_HANDLE);
these handles used?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Corinna Vinschen]

On Mar 23 21:52, Takashi Yano via Cygwin-patches wrote:
> On Tue, 23 Mar 2021 21:42:06 +0900
> Takashi Yano wrote:
> > On Tue, 23 Mar 2021 21:32:12 +0900
> > Takashi Yano wrote:
> > > I try to check run.exe behaviour and noticed that
> > > run cmd.exe
> > > and
> > > run cat.exe
> > > does not work with cygwin 3.0.7 and 3.2.0 (TEST) while these
> > > work in 3.1.7.
> > 
> > In obove cases, cmd.exe and cat.exe is running in *hidden* console,
> > therefore nothing is shown. Right?
> 
> In what situation are
>   psi->cb = sizeof (STARTUPINFO);
>   psi->hStdInput  = GetStdHandle (STD_INPUT_HANDLE);
>   psi->hStdOutput = GetStdHandle (STD_OUTPUT_HANDLE);
>   psi->hStdError  = GetStdHandle (STD_ERROR_HANDLE);
> these handles used?

Hmm, trying to make sense from the code, I'd say, these handles are used
by default, unless run.exe is already attached to a console.  In  the
latter case, it calls CreateFile( "CONIN$") etc. to attach the new
process to that console.

As Jon wrote a few minutes ago, I'm not sure what this gymnastics are
good for either...


Corinna

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Takashi Yano]

On Tue, 23 Mar 2021 14:32:39 +0100
Corinna Vinschen wrote:
> On Mar 23 21:52, Takashi Yano via Cygwin-patches wrote:
> > On Tue, 23 Mar 2021 21:42:06 +0900
> > Takashi Yano wrote:
> > > On Tue, 23 Mar 2021 21:32:12 +0900
> > > Takashi Yano wrote:
> > > > I try to check run.exe behaviour and noticed that
> > > > run cmd.exe
> > > > and
> > > > run cat.exe
> > > > does not work with cygwin 3.0.7 and 3.2.0 (TEST) while these
> > > > work in 3.1.7.
> > > 
> > > In obove cases, cmd.exe and cat.exe is running in *hidden* console,
> > > therefore nothing is shown. Right?
> > 
> > In what situation are
> >   psi->cb = sizeof (STARTUPINFO);
> >   psi->hStdInput  = GetStdHandle (STD_INPUT_HANDLE);
> >   psi->hStdOutput = GetStdHandle (STD_OUTPUT_HANDLE);
> >   psi->hStdError  = GetStdHandle (STD_ERROR_HANDLE);
> > these handles used?
> 
> Hmm, trying to make sense from the code, I'd say, these handles are used
> by default, unless run.exe is already attached to a console.  In  the
> latter case, it calls CreateFile( "CONIN$") etc. to attach the new
> process to that console.

"if (!bForceUsingPipes && bHaveConsole)"
then handles are replaced by CreateFile("CONIN$", ...) etc.
else replaced by pipe handle. If so, handles returned by
GetStdHandle() are never used.

Do I overlook something?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Corinna Vinschen]

On Mar 23 22:51, Takashi Yano wrote:
> On Tue, 23 Mar 2021 14:32:39 +0100
> Corinna Vinschen wrote:
> > On Mar 23 21:52, Takashi Yano via Cygwin-patches wrote:
> > > On Tue, 23 Mar 2021 21:42:06 +0900
> > > Takashi Yano wrote:
> > > > On Tue, 23 Mar 2021 21:32:12 +0900
> > > > Takashi Yano wrote:
> > > > > I try to check run.exe behaviour and noticed that
> > > > > run cmd.exe
> > > > > and
> > > > > run cat.exe
> > > > > does not work with cygwin 3.0.7 and 3.2.0 (TEST) while these
> > > > > work in 3.1.7.
> > > > 
> > > > In obove cases, cmd.exe and cat.exe is running in *hidden* console,
> > > > therefore nothing is shown. Right?
> > > 
> > > In what situation are
> > >   psi->cb = sizeof (STARTUPINFO);
> > >   psi->hStdInput  = GetStdHandle (STD_INPUT_HANDLE);
> > >   psi->hStdOutput = GetStdHandle (STD_OUTPUT_HANDLE);
> > >   psi->hStdError  = GetStdHandle (STD_ERROR_HANDLE);
> > > these handles used?
> > 
> > Hmm, trying to make sense from the code, I'd say, these handles are used
> > by default, unless run.exe is already attached to a console.  In  the
> > latter case, it calls CreateFile( "CONIN$") etc. to attach the new
> > process to that console.
> 
> "if (!bForceUsingPipes && bHaveConsole)"
> then handles are replaced by CreateFile("CONIN$", ...) etc.
> else replaced by pipe handle. If so, handles returned by
> GetStdHandle() are never used.
> 
> Do I overlook something?

AFAICS, bForceUsingPipes is always FALSE.  bHaveConsole is only TRUE if
run.exe is started from a console.  If run.exe is started from the GUI
(which was the primary idea, IIRC), bHaveConsole is FALSE.  So the
GetStdHandle() handles are used when started from thre GUI, isn't it?


Corinna

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Corinna Vinschen]

On Mar 23 21:32, Takashi Yano via Cygwin-patches wrote:
> On Tue, 23 Mar 2021 13:17:16 +0100
> Corinna Vinschen wrote:
> > On Mar 23 20:57, Takashi Yano via Cygwin-patches wrote:
> > > Corinna Vinschen wrote:
> > > > > > On Mar 22 08:07, Takashi Yano via Cygwin-patches wrote:
> > > > > > > > And also, following cygwin apps/dlls call GetStdHandle():
> > > > > > > > ccmake.exe
> > > > > > > > cmake.exe
> > > > > > > > cpack.exe
> > > > > > > > ctest.exe
> > > > > > > > run.exe
> > > > 
> > > > run creates its own conin/conout handles to create a hidden console.
> > > > The code calling GetStdHandle() is only for debug purposes and never
> > > > built into the executable.
> > 
> > Sorry, but this was utterly wrong.  run calls GetStdHandle, then
> > overwrites the handles, but only if it doesn't already is attached to a
> > console.
> > 
> > > > Looks right to me.  If we patch cmake to do the right thing, do we still
> > > > need this patch, Takashi?
> > > 
> > > I don't think so. If all is well with current code, nothing to be fixed.
> > 
> > How do you evaluate this in light of the run behaviour above?
> 
> I try to check run.exe behaviour and noticed that
> run cmd.exe
> and
> run cat.exe
> does not work with cygwin 3.0.7 and 3.2.0 (TEST) while these
> work in 3.1.7.
> 
> Is this expected behaviour?

The problem is that I never used run.  I can't actually tell what
exactly is expected.  I *think* run was intended to start Cygwin
applications without console window in the first place, not
native Windows apps, but I could be wrong.

I don't even know if anybody is actually, seriously using it.
The biggest problem with run is probably that it's unmaintained for
a long time, and seeing code comments like

  /* Note: we do it this way instead of using GetConsoleWindow()
   * because we want it to work on < w2k.
   */

doesn't actually make me confident in its viability for any purpose...


Corinna

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Jon Turney]

On 23/03/2021 12:44, Corinna Vinschen via Cygwin-patches wrote:
> On Mar 23 21:32, Takashi Yano via Cygwin-patches wrote:
>> On Tue, 23 Mar 2021 13:17:16 +0100
>> Corinna Vinschen wrote:
>>> On Mar 23 20:57, Takashi Yano via Cygwin-patches wrote:
>>>> Corinna Vinschen wrote:
>>>>>>> On Mar 22 08:07, Takashi Yano via Cygwin-patches wrote:
>>>>>>>>> And also, following cygwin apps/dlls call GetStdHandle():
>>>>>>>>> ccmake.exe
>>>>>>>>> cmake.exe
>>>>>>>>> cpack.exe
>>>>>>>>> ctest.exe
>>>>>>>>> run.exe
>>>>>
>>>>> run creates its own conin/conout handles to create a hidden console.
>>>>> The code calling GetStdHandle() is only for debug purposes and never
>>>>> built into the executable.
>>>
>>> Sorry, but this was utterly wrong.  run calls GetStdHandle, then
>>> overwrites the handles, but only if it doesn't already is attached to a
>>> console.
>>>
>>>>> Looks right to me.  If we patch cmake to do the right thing, do we still
>>>>> need this patch, Takashi?
>>>>
>>>> I don't think so. If all is well with current code, nothing to be fixed.
>>>
>>> How do you evaluate this in light of the run behaviour above?
>>
>> I try to check run.exe behaviour and noticed that
>> run cmd.exe
>> and
>> run cat.exe
>> does not work with cygwin 3.0.7 and 3.2.0 (TEST) while these
>> work in 3.1.7.
>>
>> Is this expected behaviour?
> 
> The problem is that I never used run.  I can't actually tell what
> exactly is expected.  I *think* run was intended to start Cygwin
> applications without console window in the first place, not
> native Windows apps, but I could be wrong.
> 
> I don't even know if anybody is actually, seriously using it.

'run' is used by the start menu item which starts the X server.

If that doesn't use it, a visible console window is created for the bash 
process it starts (which is the parent of the X server process and lives 
for it's lifetime).

(As a separate issue, I'm not sure all the complex gymnastics run does 
to creste the window invisibly are doing anything useful, since we seem 
to briefly show the window and then hide it)

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[ASSI]

Jon Turney writes:
> 'run' is used by the start menu item which starts the X server.
>
> If that doesn't use it, a visible console window is created for the
> bash process it starts (which is the parent of the X server process
> and lives for it's lifetime).
>
> (As a separate issue, I'm not sure all the complex gymnastics run does
> to creste the window invisibly are doing anything useful, since we
> seem to briefly show the window and then hide it)

It may be time to finally retire run and finish the work on run2…


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Corinna Vinschen]

On Mar 23 16:21, ASSI wrote:
> Jon Turney writes:
> > 'run' is used by the start menu item which starts the X server.
> >
> > If that doesn't use it, a visible console window is created for the
> > bash process it starts (which is the parent of the X server process
> > and lives for it's lifetime).
> >
> > (As a separate issue, I'm not sure all the complex gymnastics run does
> > to creste the window invisibly are doing anything useful, since we
> > seem to briefly show the window and then hide it)
> 
> It may be time to finally retire run and finish the work on run2…

\o/


Corinna

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Brian Inglis]

On 2021-03-23 07:09, Jon Turney wrote:
> On 23/03/2021 12:44, Corinna Vinschen via Cygwin-patches wrote:
>> On Mar 23 21:32, Takashi Yano via Cygwin-patches wrote:
>>> On Tue, 23 Mar 2021 13:17:16 +0100
>>> Corinna Vinschen wrote:
>>>> On Mar 23 20:57, Takashi Yano via Cygwin-patches wrote:
>>>>> Corinna Vinschen wrote:
>>>>>>>> On Mar 22 08:07, Takashi Yano via Cygwin-patches wrote:
>>>>>>>>>> And also, following cygwin apps/dlls call GetStdHandle():
>>>>>>>>>> ccmake.exe
>>>>>>>>>> cmake.exe
>>>>>>>>>> cpack.exe
>>>>>>>>>> ctest.exe
>>>>>>>>>> run.exe

>>>>>> run creates its own conin/conout handles to create a hidden console.
>>>>>> The code calling GetStdHandle() is only for debug purposes and never
>>>>>> built into the executable.

>>>> Sorry, but this was utterly wrong.  run calls GetStdHandle, then
>>>> overwrites the handles, but only if it doesn't already is attached to a
>>>> console.

>>>>>> Looks right to me.  If we patch cmake to do the right thing, do we still
>>>>>> need this patch, Takashi?
>>>>> I don't think so. If all is well with current code, nothing to be fixed.

>>>> How do you evaluate this in light of the run behaviour above?

>>> I try to check run.exe behaviour and noticed that
>>> run cmd.exe
>>> and
>>> run cat.exe
>>> does not work with cygwin 3.0.7 and 3.2.0 (TEST) while these
>>> work in 3.1.7.
>>> Is this expected behaviour?

>> The problem is that I never used run.  I can't actually tell what
>> exactly is expected.  I *think* run was intended to start Cygwin
>> applications without console window in the first place, not
>> native Windows apps, but I could be wrong.
>> I don't even know if anybody is actually, seriously using it.

> 'run' is used by the start menu item which starts the X server.
> 
> If that doesn't use it, a visible console window is created for the bash process 
> it starts (which is the parent of the X server process and lives for it's 
> lifetime).
> 
> (As a separate issue, I'm not sure all the complex gymnastics run does to creste 
> the window invisibly are doing anything useful, since we seem to briefly show 
> the window and then hide it)

Shortcut does:
C:\...\cygwin64\bin\run.exe --quote /usr/bin/bash.exe -l -c "cd; exec 
/usr/bin/startxwin"

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
[Data in binary units and prefixes, physical quantities in SI.]

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Marco Atzeri]

On 23.03.2021 11:10, Corinna Vinschen wrote:
> [CC Marco, CC Jan]
> 
> 
>> Out of curiosity, I took a quick glance at the cmake code.  It appears that
>> this code is designed to support running cmake in a Console.  I don't think
>> that should be needed any more, if it ever was.
>> [...]
>> I think the following might suffice (untested):
>>
>> --- a/Source/kwsys/Terminal.c
>> +++ b/Source/kwsys/Terminal.c
>> @@ -10,7 +10,7 @@
>>   #endif
>>
>>   /* Configure support for this platform.  */
>> -#if defined(_WIN32) || defined(__CYGWIN__)
>> +#if defined(_WIN32)
>>   #  define KWSYS_TERMINAL_SUPPORT_CONSOLE
>>   #endif
>>   #if !defined(_WIN32)

noted.
cmake was always annoying, we remove this type of define and they add
somewhere else


> Looks right to me.  If we patch cmake to do the right thing, do we still
> need this patch, Takashi?
> 
> 
> Thanks,
> Corinna

Regards
Marco

Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().

[Corinna Vinschen]

On Mar 24 11:43, Marco Atzeri wrote:
> On 23.03.2021 11:10, Corinna Vinschen wrote:
> > [CC Marco, CC Jan]
> > 
> > 
> > > Out of curiosity, I took a quick glance at the cmake code.  It appears that
> > > this code is designed to support running cmake in a Console.  I don't think
> > > that should be needed any more, if it ever was.
> > > [...]
> > > I think the following might suffice (untested):
> > > 
> > > --- a/Source/kwsys/Terminal.c
> > > +++ b/Source/kwsys/Terminal.c
> > > @@ -10,7 +10,7 @@
> > >   #endif
> > > 
> > >   /* Configure support for this platform.  */
> > > -#if defined(_WIN32) || defined(__CYGWIN__)
> > > +#if defined(_WIN32)
> > >   #  define KWSYS_TERMINAL_SUPPORT_CONSOLE
> > >   #endif
> > >   #if !defined(_WIN32)
> 
> noted.
> cmake was always annoying, we remove this type of define and they add
> somewhere else
> 
> 
> > Looks right to me.  If we patch cmake to do the right thing, do we still
> > need this patch, Takashi?
> > 
> > 
> > Thanks,
> > Corinna
> 
> Regards
> Marco

Thanks, Marco!


Corinna