This is the mail archive of the cygwin mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [ANNOUNCEMENT] TEST: Cygwin 3.1.0-0.3


Hi Biswapriyo and Corinna,

Thank you very much for testing.

On Fri, 30 Aug 2019 09:55:23 +0200
Corinna Vinschen wrote:
> On Aug 29 22:15, Biswapriyo Nath wrote:
> > On Thursday, August 29, 2019, Corinna Vinschen <corinna-cygwin@cygwin.com>
> > 1a. In fhandler_pty_mater::ioctl function, shouldn't the function pointer
> > be checked before use? Also what the FIXME says, isn't clear. Any hint?
> 
> Yeah, right.  What happens on pre-W10 1809 systems?  They probably
> crash on TIOCSWINSZ.  See below.

This code is protected by
if (getPseudoConsole () && ...
that is, pseudo console handle is set only if CreatePseudoConsole()
successes. In pre-W10 1809, since pseudo console handle is not set,
this code is not reached. However, it is better to check before
call as you suggest.

What is meant in FIXME comment:
ResizePseudoConsole() needs handle to pseudo console. This handle is
valid only in the process which created it. If ioctl(TIOCSWINSZ, ...)
is called from other process, it fails. I had tried DuplicateHandle(),
but it did not work. Therefore, ResizePseudoConsole() is called
only if ioctl() is called from PTY master process. Similarly,
ClosePseudoConsole() can work only in the master process.

> > 1b. GetModuleHandle and GetProcessHeap is called several times. Wouldn't be
> > it easier to get all the pseudo console function pointers in one
> > constructor?
> 
> In terms of GetModuleHandle/GetProcAddress the right thing to do is to
> use the autoload feature, i.e., add the functions to autoload.cc like
> this:
> 
>   LoadDLLfuncEx (ClosePseudoConsole, 4, kernel32, 1)
>   LoadDLLfuncEx (CreatePseudoConsole, 20, kernel32, 1)
>   LoadDLLfuncEx (ResizePseudoConsole, 8, kernel32, 1)
> 
> If the function call returns FALSE with GetLastError() ==
> ERROR_PROC_NOT_FOUND, then the function is not available.
> 
> Takashi, I didn't actually notice the usage of the Windows heap here.
> The usual method is to use a tls_pbuf buffer, and rather than
> using MultiByteToWideChar/WideCharToMultiByte, use sys_mbstowcs/
> sys_wcstombs.
> 
> Also, can you please change the camelback names in class tty_min to
> underscored writing, e.g., term_codepage rather than TermCodePage?

OK. I will revised the code followed to your advice.

First, I would like to fix some bugs and will post a patch.
Second, I will revise the coding style.

May I post them as indivisual patches? I also suppose the patch should
be against the v9 patch, right?

> > 3. I can't reproduce this issue with exact steps. But when I zoom in/out +
> > resize mintty window + execute cmd.exe in mintty in some random order it
> > crashed. Here is the error:
> > 
> > [main] D:\Cygwin64\bin\bash 1129
> > fhandler_pty_slave::push_to_pcon_screenbuffer: pty0: pcon_attach
> > mismatch?????? (0x18035DBD0)
> 
> Takashi?

This most likely is caused by a bug. Now, I am looking into this problem.
I have found the culprit, maybe.

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

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]