Re: [PATCH] Cygwin: termios: Set ECHOE, ECHOK, ECHOCTL and ECHOKE by default.

2020-05-19 Thread Brian Inglis 
On 2020-05-17 18:50, Takashi Yano wrote:
> On Mon, 18 May 2020 01:21:07 +0200, Kacper Michajlow wrote:
>> On Sun, 17 May 2020 at 04:53, Takashi Yano wrote:

>>> - Backspace key does not work correctly in linux session opend by
>>>   ssh from cygwin console if the shell is bash. This is due to lack
>>>   of these flags.
>>>
>>>   Addresses: https://cygwin.com/pipermail/cygwin/2020-May/244837.html.
>>> ---
>>>  winsup/cygwin/fhandler_termios.cc | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/winsup/cygwin/fhandler_termios.cc
>>> b/winsup/cygwin/fhandler_termios.cc
>>> index b6759b0a7..b03478b87 100644
>>> --- a/winsup/cygwin/fhandler_termios.cc
>>> +++ b/winsup/cygwin/fhandler_termios.cc
>>> @@ -33,7 +33,8 @@ fhandler_termios::tcinit (bool is_pty_master)
>>>tc ()->ti.c_iflag = BRKINT | ICRNL | IXON | IUTF8;
>>>tc ()->ti.c_oflag = OPOST | ONLCR;
>>>tc ()->ti.c_cflag = B38400 | CS8 | CREAD;
>>> -  tc ()->ti.c_lflag = ISIG | ICANON | ECHO | IEXTEN;
>>> +  tc ()->ti.c_lflag = ISIG | ICANON | ECHO | IEXTEN
>>> +   | ECHOE | ECHOK | ECHOCTL | ECHOKE;
>>>
>>>tc ()->ti.c_cc[VDISCARD] = CFLUSH;
>>>tc ()->ti.c_cc[VEOL] = CEOL;
>>> --
>>> 2.21.0

>> Maybe also set  IXANY | IMAXBEL? For reasonable set of default values.

> I don't think so, because they are not set also in xterm in linux.

IMAXBEL defaults in Cygwin mintty and Debian lxterminal:
my non-default settings are in the first output group;
all current settings are in the second output group;
these settings work with no issues across Cygwin and Linux systems;
on BSD YMMV:

Cygwin 3.1.4 mintty 3.1.4 $ stty; echo; stty -a
speed 300 baud; line = 0;
erase = ^H;
ixany iutf8

speed 300 baud; rows 60; columns 120; line = 0;
intr = ^C; quit = ^\; erase = ^H; kill = ^U; eof = ^D; eol = ; eol2 =
; swtch = ^Z; start = ^Q; stop = ^S;
susp = ^Z; rprnt = ^R; werase = ^W; lnext = ^V; discard = ^O; min = 1; time = 0;
-parenb -parodd cs8 -hupcl -cstopb cread -clocal -crtscts
-ignbrk brkint -ignpar -parmrk -inpck -istrip -inlcr -igncr icrnl ixon -ixoff
-iuclc ixany imaxbel iutf8
opost -olcuc -ocrnl onlcr -onocr -onlret -ofill -ofdel nl0 cr0 tab0 bs0 vt0 ff0
isig icanon iexten echo echoe echok -echonl -noflsh -tostop echoctl echoke 
-flusho

Debian 10.4 lxterminal 0.3.2 $ stty; echo; stty -a
speed 300 baud; line = 0;
erase = ^H; swtch = ^Z;
ixany iutf8

speed 300 baud; rows 45; columns 120; line = 0;
intr = ^C; quit = ^\; erase = ^H; kill = ^U; eof = ^D; eol = ; eol2 =
; swtch = ^Z; start = ^Q; stop = ^S;
susp = ^Z; rprnt = ^R; werase = ^W; lnext = ^V; discard = ^O; min = 1; time = 0;
-parenb -parodd -cmspar cs8 -hupcl -cstopb cread -clocal -crtscts
-ignbrk brkint -ignpar -parmrk -inpck -istrip -inlcr -igncr icrnl ixon -ixoff
-iuclc ixany imaxbel iutf8
opost -olcuc -ocrnl onlcr -onocr -onlret -ofill -ofdel nl0 cr0 tab0 bs0 vt0 ff0
isig icanon iexten echo echoe echok -echonl -noflsh -xcase -tostop -echoprt
echoctl echoke -flusho -extproc

-- 
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 IEC units and prefixes, physical quantities in SI.]

Re: [PATCH] Cygwin: pty: Make system_printf() work after closing pty slave.

2020-05-19 Thread Ken Brown via Cygwin-patches 

Hi Takashi,

On 5/19/2020 7:35 AM, Takashi Yano via Cygwin-patches wrote:

- Current pty cannot show system_printf() output after closing pty
   slave. This patch fixes the issue.


Sorry to be returning the favor so soon, but this patch causes 'make check' in 
the texinfo source tree to hang.  I don't have time at the moment to try to 
produce a simple test case, so here's a complicated way to reproduce the problem:


1. Clone the texinfo git repo:

  $ git clone https://git.savannah.gnu.org/git/texinfo.git

2. Build texinfo:

  $ cd texinfo
  $ ./autogen.sh && ./configure # Maybe CFLAGS='-g -O0' for debugging
  $ make

3. Test the standalone info reader:

  $ cd info
  $ make check

It hangs while running the test t/malformed-split.sh, leaving a ginfo process 
and a pseudotty process running, with ginfo trying to close a pty slave.


Note that this test uses both ptys and fifos, so there's always a chance that 
this is another fifo bug.  But reverting your patch fixes the problem, so I 
think it's probably a pty bug.


Ken

Re: [PATCH 00/21] FIFO: Support multiple readers

2020-05-19 Thread Takashi Yano via Cygwin-patches 
On Tue, 19 May 2020 09:37:17 -0400
Ken Brown via Cygwin-patches  wrote:
> On 5/19/2020 8:51 AM, Ken Brown via Cygwin-patches wrote:
> > On 5/19/2020 2:15 AM, Takashi Yano via Cygwin-patches wrote:
> >> On Tue, 19 May 2020 10:26:09 +0900
> >> Takashi Yano via Cygwin-patches  wrote:
> >>> Hi Ken,
> >>>
> >>> On Mon, 18 May 2020 13:42:19 -0400
> >>> Ken Brown via Cygwin-patches  wrote:
>  Hi Takashi,
> 
>  On 5/18/2020 12:03 PM, Ken Brown via Cygwin-patches wrote:
> > On 5/18/2020 1:36 AM, Takashi Yano via Cygwin-patches wrote:
> >> On Mon, 18 May 2020 14:25:19 +0900
> >> Takashi Yano via Cygwin-patches  wrote:
> >>> However, mc hangs by several operations.
> >>>
> >>> To reproduce this:
> >>> 1. Start mc with 'env SHELL=tcsh mc -a'
> >>
> >> I mean 'env SHELL=/bin/tcsh mc -a'
> >>
> >>> 2. Select a file using up/down cursor keys.
> >>> 3. Press F3 (View) key.
> >
> > Thanks for the report.  I can reproduce the problem and will look into 
> > it.
> 
>  I'm not convinced that this is a FIFO bug.  I tried two things.
> 
>  1. I attached gdb to mc while it was hanging and got the following 
>  backtrace
>  (abbreviated):
> 
>  #1  0x7ff901638037 in WaitForMultipleObjectsEx ()
>  #2  0x7ff901637f1e in WaitForMultipleObjects ()
>  #3  0x000180048df5 in cygwait () at ...winsup/cygwin/cygwait.cc:75
>  #4  0x00018019b1c0 in wait4 () at ...winsup/cygwin/wait.cc:80
>  #5  0x00018019afea in waitpid () at ...winsup/cygwin/wait.cc:28
>  #6  0x00018017d2d8 in pclose () at ...winsup/cygwin/syscalls.cc:4627
>  #7  0x00018015943b in _sigfe () at sigfe.s:35
>  #8  0x00010040d002 in get_popen_information () at 
>  filemanager/ext.c:561
>  [...]
> 
>  So pclose is blocking after calling waitpid.  As far as I can tell from 
>  looking
>  at backtraces of all threads, there are no FIFOs open.
> 
>  2. I ran mc under strace (after exporting SHELL=/bin/tcsh), and I didn't 
>  see
>  anything suspicious involving FIFOs.  But I saw many EBADF errors from 
>  fstat 
>  and
>  close that don't appear to be related to FIFOs.
> 
>  So my best guess at this point is that the FIFO changes just exposed some
>  unrelated bug(s).
> 
>  Prior to the FIFO changes, mc would get an error when it tried to open 
>  tcsh_fifo
>  the second time, and it would then set
> 
>      mc_global.tty.use_subshell = FALSE;
> 
>  see the mc source file subshell/common.c:1087.
> >>>
> >>> I looked into this problem and found pclose() stucks if FIFO
> >>> is opened.
> >>>
> >>> Attached is a simple test case. It works under cygwin 3.1.4,
> >>> but stucks at pclose() under cygwin git head.
> >>>
> >>> Isn't this a FIFO related issue?
> >>
> >> In the test case, fhandler_fifo::close() called from /bin/true
> >> seems to get into infinite loop at:
> >>
> >> do
> >> ...
> >> while (inc_nreaders () > 0 && !found);
> > 
> > Thank you!  I see the problem.  After the popen call, the original 
> > fhandler_fifo's fifo_reader_thread was no longer running, so it was unable 
> > to 
> > take ownership.
> > 
> > It might take a little while for me to figure out how to fix this.
> 
> Actually, I think it's easy.  Please try the two attached patches.  The 
> second 
> one is the crucial one for the mc problem, but the first is something I 
> noticed 
> while working on this.
> 
> I just did a quick and dirty patch for testing purposes.  I still have to do 
> a 
> lot of cleanup and make sure the fix doesn't break something else.

For a shor time, I tested these patches, and confirmed
that this fixes the problem.

Thanks for the quick response.

-- 
Takashi Yano

Re: [PATCH 00/21] FIFO: Support multiple readers

2020-05-19 Thread Ken Brown via Cygwin-patches 

On 5/19/2020 8:51 AM, Ken Brown via Cygwin-patches wrote:

On 5/19/2020 2:15 AM, Takashi Yano via Cygwin-patches wrote:

On Tue, 19 May 2020 10:26:09 +0900
Takashi Yano via Cygwin-patches  wrote:

Hi Ken,

On Mon, 18 May 2020 13:42:19 -0400
Ken Brown via Cygwin-patches  wrote:

Hi Takashi,

On 5/18/2020 12:03 PM, Ken Brown via Cygwin-patches wrote:

On 5/18/2020 1:36 AM, Takashi Yano via Cygwin-patches wrote:

On Mon, 18 May 2020 14:25:19 +0900
Takashi Yano via Cygwin-patches  wrote:

However, mc hangs by several operations.

To reproduce this:
1. Start mc with 'env SHELL=tcsh mc -a'


I mean 'env SHELL=/bin/tcsh mc -a'


2. Select a file using up/down cursor keys.
3. Press F3 (View) key.


Thanks for the report.  I can reproduce the problem and will look into it.


I'm not convinced that this is a FIFO bug.  I tried two things.

1. I attached gdb to mc while it was hanging and got the following backtrace
(abbreviated):

#1  0x7ff901638037 in WaitForMultipleObjectsEx ()
#2  0x7ff901637f1e in WaitForMultipleObjects ()
#3  0x000180048df5 in cygwait () at ...winsup/cygwin/cygwait.cc:75
#4  0x00018019b1c0 in wait4 () at ...winsup/cygwin/wait.cc:80
#5  0x00018019afea in waitpid () at ...winsup/cygwin/wait.cc:28
#6  0x00018017d2d8 in pclose () at ...winsup/cygwin/syscalls.cc:4627
#7  0x00018015943b in _sigfe () at sigfe.s:35
#8  0x00010040d002 in get_popen_information () at filemanager/ext.c:561
[...]

So pclose is blocking after calling waitpid.  As far as I can tell from looking
at backtraces of all threads, there are no FIFOs open.

2. I ran mc under strace (after exporting SHELL=/bin/tcsh), and I didn't see
anything suspicious involving FIFOs.  But I saw many EBADF errors from fstat 
and

close that don't appear to be related to FIFOs.

So my best guess at this point is that the FIFO changes just exposed some
unrelated bug(s).

Prior to the FIFO changes, mc would get an error when it tried to open 
tcsh_fifo

the second time, and it would then set

    mc_global.tty.use_subshell = FALSE;

see the mc source file subshell/common.c:1087.


I looked into this problem and found pclose() stucks if FIFO
is opened.

Attached is a simple test case. It works under cygwin 3.1.4,
but stucks at pclose() under cygwin git head.

Isn't this a FIFO related issue?


In the test case, fhandler_fifo::close() called from /bin/true
seems to get into infinite loop at:

do
...
while (inc_nreaders () > 0 && !found);


Thank you!  I see the problem.  After the popen call, the original 
fhandler_fifo's fifo_reader_thread was no longer running, so it was unable to 
take ownership.


It might take a little while for me to figure out how to fix this.


Actually, I think it's easy.  Please try the two attached patches.  The second 
one is the crucial one for the mc problem, but the first is something I noticed 
while working on this.


I just did a quick and dirty patch for testing purposes.  I still have to do a 
lot of cleanup and make sure the fix doesn't break something else.


Ken
>From bdc0bd172b09d386a2f15c41e7a764f48748b90c Mon Sep 17 00:00:00 2001
From: Ken Brown 
Date: Tue, 19 May 2020 09:03:59 -0400
Subject: [PATCH 1/2] Cygwin: FIFO: add missing reader_opening_unlock

---
 winsup/cygwin/fhandler_fifo.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index e8a05dfbf..0dc356dfe 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -1047,6 +1047,7 @@ err_close_reader:
   saved_errno = get_errno ();
   close ();
   set_errno (saved_errno);
+  reader_opening_unlock ();
   return 0;
 err_close_cancel_evt:
   NtClose (cancel_evt);
-- 
2.21.0

>From ce60aa56a573e712ce5d212ca6aa0ddd9781be29 Mon Sep 17 00:00:00 2001
From: Ken Brown 
Date: Tue, 19 May 2020 09:28:30 -0400
Subject: [PATCH 2/2] Cygwin: FIFO: don't take ownership after exec

Proof of concept only.  Needs cleanup and further testing.

There's no need to transfer ownership after exec.  This will happen
automatically, if necessary, when the parent closes.  And canceling
the parent's fifo_reader_thread can cause problems, as reported here
for example:

https://cygwin.com/pipermail/cygwin-patches/2020q2/010235.html
---
 winsup/cygwin/fhandler_fifo.cc | 35 +-
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 0dc356dfe..79f83177f 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -1727,23 +1727,23 @@ fhandler_fifo::fixup_after_exec ()
   fc_handler = NULL;
   nhandlers = shandlers = 0;
 
-  /* Cancel parent's reader thread */
-  if (cancel_evt)
-   SetEvent (cancel_evt);
-  if (thr_sync_evt)
-   WaitForSingleObject (thr_sync_evt, INFINITE);
-
-  /* Take ownership if parent is owner. */
-  fifo_reader_id_t parent_fr = me;
-  me.winpid = GetCurrentProcessId ();
-

Re: [PATCH] Cygwin: pty: Make system_printf() work after closing pty slave.

2020-05-19 Thread Corinna Vinschen 
On May 19 20:35, Takashi Yano via Cygwin-patches wrote:
> - Current pty cannot show system_printf() output after closing pty
>   slave. This patch fixes the issue.
> ---
>  winsup/cygwin/fhandler_tty.cc | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> index 5a1bcd3ce..02b78cd2c 100644
> --- a/winsup/cygwin/fhandler_tty.cc
> +++ b/winsup/cygwin/fhandler_tty.cc
> @@ -948,6 +948,10 @@ fhandler_pty_slave::open (int flags, mode_t)
>init_console_handler (true);
>  }
>  
> +  get_ttyp ()->pcon_pid = 0;
> +  get_ttyp ()->switch_to_pcon_in = false;
> +  get_ttyp ()->switch_to_pcon_out = false;
> +
>set_open_status ();
>return 1;
>  
> @@ -1008,6 +1012,7 @@ fhandler_pty_slave::close ()
>  termios_printf ("CloseHandle (output_mutex<%p>), %E", output_mutex);
>if (pcon_attached_to == get_minor ())
>  get_ttyp ()->num_pcon_attached_slaves --;
> +  set_switch_to_pcon (2); /* Make system_printf() work after close. */
>return 0;
>  }
>  
> -- 
> 2.21.0

Pushed.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

Re: [PATCH v2] Cygwin: pty: Call FreeConsole() only if attached to current pty.

2020-05-19 Thread Corinna Vinschen 
On May 19 19:55, Takashi Yano via Cygwin-patches wrote:
> - After commit 071b8e0cbd4be33449c12bb0d58f514ed8ef893c, the problem
>   reported in https://cygwin.com/pipermail/cygwin/2020-May/244873.html
>   occurs. This is due to freeing console device accidentally rather
>   than pseudo console. This patch makes sure to call FreeConsole()
>   only if the process is attached to the pseudo console of the current
>   pty.
> ---
>  winsup/cygwin/fhandler.h  |  1 +
>  winsup/cygwin/fhandler_tty.cc | 28 +---
>  2 files changed, 14 insertions(+), 15 deletions(-)

Pushed.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

Re: [PATCH 00/21] FIFO: Support multiple readers

2020-05-19 Thread Ken Brown via Cygwin-patches 

On 5/19/2020 2:15 AM, Takashi Yano via Cygwin-patches wrote:

On Tue, 19 May 2020 10:26:09 +0900
Takashi Yano via Cygwin-patches  wrote:

Hi Ken,

On Mon, 18 May 2020 13:42:19 -0400
Ken Brown via Cygwin-patches  wrote:

Hi Takashi,

On 5/18/2020 12:03 PM, Ken Brown via Cygwin-patches wrote:

On 5/18/2020 1:36 AM, Takashi Yano via Cygwin-patches wrote:

On Mon, 18 May 2020 14:25:19 +0900
Takashi Yano via Cygwin-patches  wrote:

However, mc hangs by several operations.

To reproduce this:
1. Start mc with 'env SHELL=tcsh mc -a'


I mean 'env SHELL=/bin/tcsh mc -a'


2. Select a file using up/down cursor keys.
3. Press F3 (View) key.


Thanks for the report.  I can reproduce the problem and will look into it.


I'm not convinced that this is a FIFO bug.  I tried two things.

1. I attached gdb to mc while it was hanging and got the following backtrace
(abbreviated):

#1  0x7ff901638037 in WaitForMultipleObjectsEx ()
#2  0x7ff901637f1e in WaitForMultipleObjects ()
#3  0x000180048df5 in cygwait () at ...winsup/cygwin/cygwait.cc:75
#4  0x00018019b1c0 in wait4 () at ...winsup/cygwin/wait.cc:80
#5  0x00018019afea in waitpid () at ...winsup/cygwin/wait.cc:28
#6  0x00018017d2d8 in pclose () at ...winsup/cygwin/syscalls.cc:4627
#7  0x00018015943b in _sigfe () at sigfe.s:35
#8  0x00010040d002 in get_popen_information () at filemanager/ext.c:561
[...]

So pclose is blocking after calling waitpid.  As far as I can tell from looking
at backtraces of all threads, there are no FIFOs open.

2. I ran mc under strace (after exporting SHELL=/bin/tcsh), and I didn't see
anything suspicious involving FIFOs.  But I saw many EBADF errors from fstat and
close that don't appear to be related to FIFOs.

So my best guess at this point is that the FIFO changes just exposed some
unrelated bug(s).

Prior to the FIFO changes, mc would get an error when it tried to open tcsh_fifo
the second time, and it would then set

mc_global.tty.use_subshell = FALSE;

see the mc source file subshell/common.c:1087.


I looked into this problem and found pclose() stucks if FIFO
is opened.

Attached is a simple test case. It works under cygwin 3.1.4,
but stucks at pclose() under cygwin git head.

Isn't this a FIFO related issue?


In the test case, fhandler_fifo::close() called from /bin/true
seems to get into infinite loop at:

do
...
while (inc_nreaders () > 0 && !found);


Thank you!  I see the problem.  After the popen call, the original 
fhandler_fifo's fifo_reader_thread was no longer running, so it was unable to 
take ownership.


It might take a little while for me to figure out how to fix this.

Thanks for your persistence and, especially, for the test case.

Ken

Re: [PATCH 3/3] Cygwin: tzcode resync v2: details

2020-05-19 Thread Corinna Vinschen 
Hi Mark,

On May 18 22:02, Mark Geisert wrote:
> Add tz_posixrules.h with data generated from most recent Cygwin tzdata
> package.  Establish localtime.cc as primarily a wrapper around a patched
> copy of localtime.c.  See README for more information.

The idea is nice, but there are a few problems.

> diff --git a/winsup/cygwin/tzcode/localtime.c.patch 
> b/winsup/cygwin/tzcode/localtime.c.patch
> new file mode 100644
> index 0..a17d9ee90
> --- /dev/null
> +++ b/winsup/cygwin/tzcode/localtime.c.patch
> @@ -0,0 +1,399 @@
> +*** localtime.c  2020-05-16 21:54:00.533111800 -0700
> +--- localtime.c.patched  2020-05-16 22:42:40.486924300 -0700
> +***
> +*** 1,3 

Ideally this patch should be in unified (-u) diff format.

I also wonder if some of these patches may be dropped or made
less invasive, given that localtime.cc wraps localtime.c.

For example:

> +***
> +*** 23,29 
> +  
> +  /*LINTLIBRARY*/
> +  
> +- #include "namespace.h"

Adding a local namesapce.h file?

> +  #include 
> +  #define LOCALTIME_IMPLEMENTATION
> +  #include "private.h"
> +--- 24,29 
> +***
> +*** 182,188 
> +  
> +  
> +  #if !defined(__LIBC12_SOURCE__)
> +! timezone_t __lclptr;
> +  #ifdef _REENTRANT
> +  rwlock_t __lcl_lock = RWLOCK_INITIALIZER;
> +  #endif
> +--- 182,188 
> +  
> +  
> +  #if !defined(__LIBC12_SOURCE__)
> +! static timezone_t __lclptr;

Do we really need this static?  Even if it's not static, it won't
be exported from the DLL anyway, so it seems we can drop this, no?

> +  #ifdef _REENTRANT
> +  rwlock_t __lcl_lock = RWLOCK_INITIALIZER;
> +  #endif
> +***
> +*** 198,204 
> +  
> +  static struct tm   tm;
> +  
> +! #if !HAVE_POSIX_DECLS || TZ_TIME_T || defined(__NetBSD__)
> +  # if !defined(__LIBC12_SOURCE__)
> +  
> +  __aconst char *tzname[2] = {
> +--- 198,204 
> +  
> +  static struct tm   tm;
> +  
> +! #if !HAVE_POSIX_DECLS || TZ_TIME_T || defined(__NetBSD__) || 
> defined(__CYGWIN__)

What about just #defining TZ_TIME_T or HAVE_POSIX_DECLS in localtime.cc
or even (see above) namespace.h?

> +  # if !defined(__LIBC12_SOURCE__)
> +  
> +  __aconst char *tzname[2] = {
> +***
> +*** 413,419 
> +  };
> +  
> +  /* TZDIR with a trailing '/' rather than a trailing '\0'.  */
> +! static char const tzdirslash[sizeof TZDIR] = TZDIR "/";

Yay, this one is tricky (and s dumb)

> +  
> +  /* Local storage needed for 'tzloadbody'.  */
> +  union local_storage {
> +--- 413,420 
> +  };
> +  
> +  /* TZDIR with a trailing '/' rather than a trailing '\0'.  */
> +! static char const tzdirslash[] = TZDIR "/";
> +! #define sizeof_tzdirslash (sizeof tzdirslash - 1)

What about this instead:

  - static char const tzdirslash[sizeof TZDIR] = TZDIR "/";
  + static char const tzdirslash[sizeof TZDIR + 1] = TZDIR "/";

checking what "sizeof tzdirslash" is used for, it doesn't matter
at all:

> +***
> +*** 428,434 
> +  
> + /* The file name to be opened.  */
> + char fullname[/*CONSTCOND*/BIGGEST(sizeof (struct file_analysis),
> +!sizeof tzdirslash + 1024)];

This makes fullname one byte longer than in NetBSD...

> +*** 466,479 
> + if (!doaccess) {
> + char const *dot;
> + size_t namelen = strlen(name);
> +!if (sizeof lsp->fullname - sizeof tzdirslash <= namelen)

lsp->fullname is one byte longer, tzdirslash is one byte longer,
therefore

  Cygwin:  sizeof lsp->fullname - sizeof tzdirslash

is equivalent to 

  NetBSD: (sizeof lsp->fullname + 1) - (sizeof tzdirslash + 1)
  ==   sizeof lsp->fullname + 1 - sizeof tzdirslash - 1
  ==   sizeof lsp->fullname - sizeof tzdirslash

So it's in fact the same expression and no change is required.

> + return ENAMETOOLONG;
> +  
> + /* Create a string "TZDIR/NAME".  Using sprintf here
> +would pull in stdio (and would fail if the
> +resulting string length exceeded INT_MAX!).  */
> +!memcpy(lsp->fullname, tzdirslash, sizeof tzdirslash);
> +!strcpy(lsp->fullname + sizeof tzdirslash, name);

This could then be changed to just

  - strcpy(lsp->fullname + sizeof tzdirslash, name);
  + strcpy(lsp->fullname + sizeof tzdirslash - 1, name);

> +***
> +*** 488,498 
> + name = lsp->fullname;
> + }
> + if (doaccess && access(name, R_OK) != 0)
> +!return errno;
> +  
> + fid = open(name, OPEN_MODE);
> + if (fid < 0)
> +!return errno;
> + nread = read(fid, up->buf, sizeof up->buf);
> + if (nread < (ssize_t)tzheadsize) {
> + int err = nread < 0 ? errno : EINVAL;
> +--- 489,499 
> + name = lsp->fullname;
> + }
> + if (doaccess && access(name, R_OK) != 0)
> +!goto trydefrules;
> +  
> + fid = open(name, OPEN_MODE);
> + if (fid < 0)
> +!

[PATCH] Cygwin: pty: Make system_printf() work after closing pty slave.

2020-05-19 Thread Takashi Yano via Cygwin-patches 
- Current pty cannot show system_printf() output after closing pty
  slave. This patch fixes the issue.
---
 winsup/cygwin/fhandler_tty.cc | 5 +
 1 file changed, 5 insertions(+)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 5a1bcd3ce..02b78cd2c 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -948,6 +948,10 @@ fhandler_pty_slave::open (int flags, mode_t)
   init_console_handler (true);
 }
 
+  get_ttyp ()->pcon_pid = 0;
+  get_ttyp ()->switch_to_pcon_in = false;
+  get_ttyp ()->switch_to_pcon_out = false;
+
   set_open_status ();
   return 1;
 
@@ -1008,6 +1012,7 @@ fhandler_pty_slave::close ()
 termios_printf ("CloseHandle (output_mutex<%p>), %E", output_mutex);
   if (pcon_attached_to == get_minor ())
 get_ttyp ()->num_pcon_attached_slaves --;
+  set_switch_to_pcon (2); /* Make system_printf() work after close. */
   return 0;
 }
 
-- 
2.21.0

[PATCH v2] Cygwin: pty: Call FreeConsole() only if attached to current pty.

2020-05-19 Thread Takashi Yano via Cygwin-patches 
- After commit 071b8e0cbd4be33449c12bb0d58f514ed8ef893c, the problem
  reported in https://cygwin.com/pipermail/cygwin/2020-May/244873.html
  occurs. This is due to freeing console device accidentally rather
  than pseudo console. This patch makes sure to call FreeConsole()
  only if the process is attached to the pseudo console of the current
  pty.
---
 winsup/cygwin/fhandler.h  |  1 +
 winsup/cygwin/fhandler_tty.cc | 28 +---
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index ae64086df..857f0a4e0 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2284,6 +2284,7 @@ class fhandler_pty_slave: public fhandler_pty_common
 
   bool try_reattach_pcon ();
   void restore_reattach_pcon ();
+  inline void free_attached_console ();
 
  public:
   /* Constructor */
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 8547ec7c4..5a1bcd3ce 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -180,6 +180,16 @@ set_ishybrid_and_switch_to_pcon (HANDLE h)
 }
 }
 
+inline void
+fhandler_pty_slave::free_attached_console ()
+{
+  if (freeconsole_on_close && get_minor () == pcon_attached_to)
+{
+  FreeConsole ();
+  pcon_attached_to = -1;
+}
+}
+
 #define DEF_HOOK(name) static __typeof__ (name) *name##_Orig
 DEF_HOOK (WriteFile);
 DEF_HOOK (WriteConsoleA);
@@ -708,11 +718,7 @@ fhandler_pty_slave::~fhandler_pty_slave ()
   if (!get_ttyp ())
 {
   /* Why comes here? Who clears _tc? */
-  if (freeconsole_on_close)
-   {
- FreeConsole ();
- pcon_attached_to = -1;
-   }
+  free_attached_console ();
   return;
 }
   if (get_pseudo_console ())
@@ -739,11 +745,7 @@ fhandler_pty_slave::~fhandler_pty_slave ()
   if (used == 0)
{
  init_console_handler (false);
- if (freeconsole_on_close)
-   {
- FreeConsole ();
- pcon_attached_to = -1;
-   }
+ free_attached_console ();
}
 }
 }
@@ -3006,11 +3008,7 @@ fhandler_pty_slave::fixup_after_exec ()
   if (used == 1 /* About to close this tty */)
{
  init_console_handler (false);
- if (freeconsole_on_close)
-   {
- FreeConsole ();
- pcon_attached_to = -1;
-   }
+ free_attached_console ();
}
 }
 
-- 
2.21.0

Re: Re [Cygwin PATCH */9] tzcode resync -- for discussion only

2020-05-19 Thread Corinna Vinschen 
On May 19 11:48, Corinna Vinschen wrote:
> On May 13 15:40, Mark Geisert wrote:
> > I'm not absolutely sure yet but I think this patch set isn't complete.
> > What's been posted is OK for discussion (on cygwin-developers?) but would
> > need to be augmented if you're going to apply as-is.
> > 
> > I had a git commit/revert mishap and these are recovered file versions.  I
> > belatedly discovered there's no record in the patches of my creating
> > directory winsup/cygwin/tzcode or of my deleting localtime.cc and
> > tz_posixrules.h from winsup/cygwin in my local repository.
> > 
> > When it becomes time to submit the final patch versions, I'll do so from a
> > brand new repository.  Sorry for any confusion.
> 
> Patchset discussions should stay on cygwin-patches as part of the set.
> 
> Please resend the patchset in the current state on top of current git.
> Please add a cover letter (git format-patch --cover-letter ...) and just
> start the discussion yourself in the cover letter.  Note what you
> think is missing from the patchset and what you think needs addition or
> change, so we're vaguely on the same level as you :)

Scratch that, I didn't see your v2 yet, sorry!


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

Re: [PATCH] Cygwin: termios: Set ECHOE, ECHOK, ECHOCTL and ECHOKE by default.

2020-05-19 Thread Corinna Vinschen 
On May 17 11:34, Takashi Yano via Cygwin-patches wrote:
> - Backspace key does not work correctly in linux session opend by
>   ssh from cygwin console if the shell is bash. This is due to lack
>   of these flags.
> 
>   Addresses: https://cygwin.com/pipermail/cygwin/2020-May/244837.html.
> ---
>  winsup/cygwin/fhandler_termios.cc | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/winsup/cygwin/fhandler_termios.cc 
> b/winsup/cygwin/fhandler_termios.cc
> index b6759b0a7..b03478b87 100644
> --- a/winsup/cygwin/fhandler_termios.cc
> +++ b/winsup/cygwin/fhandler_termios.cc
> @@ -33,7 +33,8 @@ fhandler_termios::tcinit (bool is_pty_master)
>tc ()->ti.c_iflag = BRKINT | ICRNL | IXON | IUTF8;
>tc ()->ti.c_oflag = OPOST | ONLCR;
>tc ()->ti.c_cflag = B38400 | CS8 | CREAD;
> -  tc ()->ti.c_lflag = ISIG | ICANON | ECHO | IEXTEN;
> +  tc ()->ti.c_lflag = ISIG | ICANON | ECHO | IEXTEN
> + | ECHOE | ECHOK | ECHOCTL | ECHOKE;
>  
>tc ()->ti.c_cc[VDISCARD]   = CFLUSH;
>tc ()->ti.c_cc[VEOL]   = CEOL;
> -- 
> 2.21.0

Pushed.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

Re: [PATCH] Cygwin: pty: Call FreeConsole() only if attached to current pty.

2020-05-19 Thread Corinna Vinschen 
On May 17 11:34, Takashi Yano via Cygwin-patches wrote:
> - After commit 071b8e0cbd4be33449c12bb0d58f514ed8ef893c, the problem
>   reported in https://cygwin.com/pipermail/cygwin/2020-May/244873.html
>   occurs. This is due to freeing console device accidentally rather
>   than pseudo console. This patch makes sure to call FreeConsole()
>   only if the process is attached to the pseudo console of the current
>   pty.
> ---
>  winsup/cygwin/fhandler_tty.cc | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> index 8547ec7c4..467784255 100644
> --- a/winsup/cygwin/fhandler_tty.cc
> +++ b/winsup/cygwin/fhandler_tty.cc
> @@ -708,7 +708,7 @@ fhandler_pty_slave::~fhandler_pty_slave ()
>if (!get_ttyp ())
>  {
>/* Why comes here? Who clears _tc? */
> -  if (freeconsole_on_close)
> +  if (freeconsole_on_close && get_minor () == pcon_attached_to)
>   {
> FreeConsole ();
> pcon_attached_to = -1;

Given it's used three times, wouldn't it make sense to put the entire
"if" construct into a fhandler_pty_slave inline method?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

Re: [PATCH] libc: Replace i386/sys/fenv.h symlink with an #include shim

2020-05-19 Thread Corinna Vinschen 
On May 16 14:43, David Macek via Cygwin-patches wrote:
> Same reasoning as fbaa0967.

This patch should go to the newlib mailing list, not here.  Please add a
bit more context than just the SHA-1 in the commit message and ideally
plewase CC Joel Sherrill, the author of the original patch.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

Re: Re [Cygwin PATCH */9] tzcode resync -- for discussion only

2020-05-19 Thread Corinna Vinschen 
On May 13 15:40, Mark Geisert wrote:
> I'm not absolutely sure yet but I think this patch set isn't complete.
> What's been posted is OK for discussion (on cygwin-developers?) but would
> need to be augmented if you're going to apply as-is.
> 
> I had a git commit/revert mishap and these are recovered file versions.  I
> belatedly discovered there's no record in the patches of my creating
> directory winsup/cygwin/tzcode or of my deleting localtime.cc and
> tz_posixrules.h from winsup/cygwin in my local repository.
> 
> When it becomes time to submit the final patch versions, I'll do so from a
> brand new repository.  Sorry for any confusion.

Patchset discussions should stay on cygwin-patches as part of the set.

Please resend the patchset in the current state on top of current git.
Please add a cover letter (git format-patch --cover-letter ...) and just
start the discussion yourself in the cover letter.  Note what you
think is missing from the patchset and what you think needs addition or
change, so we're vaguely on the same level as you :)


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

Re: [PATCH v2] cygwin: doc: Add keywords for ACE order issues

2020-05-19 Thread Corinna Vinschen 
On May 13 17:34, David Macek via Cygwin-patches wrote:
> Windows Explorer shows a warning with Cygwin-created DACLs, but putting
> the text of the warning into Google doesn't lead to the relevant Cygwin
> docs.  Let's copy the warning text into the docs in the hopes of helping
> confused users.  Most of the credit for the wording belongs to Yaakov
> Selkowitz.
> 
> Latest inquiry: 
> 
> Signed-off-by: David Macek 
> ---

Pushed.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

Re: [PATCH 00/21] FIFO: Support multiple readers

2020-05-19 Thread Takashi Yano via Cygwin-patches 
On Tue, 19 May 2020 10:26:09 +0900
Takashi Yano via Cygwin-patches  wrote:
> Hi Ken,
> 
> On Mon, 18 May 2020 13:42:19 -0400
> Ken Brown via Cygwin-patches  wrote:
> > Hi Takashi,
> > 
> > On 5/18/2020 12:03 PM, Ken Brown via Cygwin-patches wrote:
> > > On 5/18/2020 1:36 AM, Takashi Yano via Cygwin-patches wrote:
> > >> On Mon, 18 May 2020 14:25:19 +0900
> > >> Takashi Yano via Cygwin-patches  wrote:
> > >>> However, mc hangs by several operations.
> > >>>
> > >>> To reproduce this:
> > >>> 1. Start mc with 'env SHELL=tcsh mc -a'
> > >>
> > >> I mean 'env SHELL=/bin/tcsh mc -a'
> > >>
> > >>> 2. Select a file using up/down cursor keys.
> > >>> 3. Press F3 (View) key.
> > > 
> > > Thanks for the report.  I can reproduce the problem and will look into it.
> > 
> > I'm not convinced that this is a FIFO bug.  I tried two things.
> > 
> > 1. I attached gdb to mc while it was hanging and got the following 
> > backtrace 
> > (abbreviated):
> > 
> > #1  0x7ff901638037 in WaitForMultipleObjectsEx ()
> > #2  0x7ff901637f1e in WaitForMultipleObjects ()
> > #3  0x000180048df5 in cygwait () at ...winsup/cygwin/cygwait.cc:75
> > #4  0x00018019b1c0 in wait4 () at ...winsup/cygwin/wait.cc:80
> > #5  0x00018019afea in waitpid () at ...winsup/cygwin/wait.cc:28
> > #6  0x00018017d2d8 in pclose () at ...winsup/cygwin/syscalls.cc:4627
> > #7  0x00018015943b in _sigfe () at sigfe.s:35
> > #8  0x00010040d002 in get_popen_information () at filemanager/ext.c:561
> > [...]
> > 
> > So pclose is blocking after calling waitpid.  As far as I can tell from 
> > looking 
> > at backtraces of all threads, there are no FIFOs open.
> > 
> > 2. I ran mc under strace (after exporting SHELL=/bin/tcsh), and I didn't 
> > see 
> > anything suspicious involving FIFOs.  But I saw many EBADF errors from 
> > fstat and 
> > close that don't appear to be related to FIFOs.
> > 
> > So my best guess at this point is that the FIFO changes just exposed some 
> > unrelated bug(s).
> > 
> > Prior to the FIFO changes, mc would get an error when it tried to open 
> > tcsh_fifo 
> > the second time, and it would then set
> > 
> >mc_global.tty.use_subshell = FALSE;
> > 
> > see the mc source file subshell/common.c:1087.
> 
> I looked into this problem and found pclose() stucks if FIFO
> is opened.
> 
> Attached is a simple test case. It works under cygwin 3.1.4,
> but stucks at pclose() under cygwin git head.
> 
> Isn't this a FIFO related issue?

In the test case, fhandler_fifo::close() called from /bin/true
seems to get into infinite loop at:

do
...
while (inc_nreaders () > 0 && !found);


-- 
Takashi Yano