[PATCH] Cygwin: console: Ignore 0x00 on write().

2020-02-20 Thread Takashi Yano
- In xterm compatible mode, 0x00 on write() behaves incompatible
  with real xterm. In xterm, 0x00 completely ignored. Therefore,
  0x00 is ignored by console with this patch.
---
 winsup/cygwin/fhandler_console.cc | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/winsup/cygwin/fhandler_console.cc 
b/winsup/cygwin/fhandler_console.cc
index 66e645aa1..705ce696e 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -1794,6 +1794,16 @@ bool fhandler_console::write_console (PWCHAR buf, DWORD 
len, DWORD& done)
  len -= 4;
}
 }
+  /* Workaround for ^@ (0x00) handling in xterm compatible mode. */
+  if (wincap.has_con_24bit_colors () && !con_is_legacy)
+{
+  WCHAR *p = buf;
+  while ((p = wmemchr (p, L'\0', len - (p - buf
+   {
+ memmove (p, p+1, (len - (p+1 - buf))*sizeof (WCHAR));
+ len --;
+   }
+}
 
   if (con.iso_2022_G1
? con.vt100_graphics_mode_G1
-- 
2.21.0



Re: [PATCH] Cygwin: console: Ignore 0x00 on write().

2020-02-20 Thread Corinna Vinschen
On Feb 20 20:51, Takashi Yano wrote:
> - In xterm compatible mode, 0x00 on write() behaves incompatible
>   with real xterm. In xterm, 0x00 completely ignored. Therefore,
>   0x00 is ignored by console with this patch.
> ---
>  winsup/cygwin/fhandler_console.cc | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/winsup/cygwin/fhandler_console.cc 
> b/winsup/cygwin/fhandler_console.cc
> index 66e645aa1..705ce696e 100644
> --- a/winsup/cygwin/fhandler_console.cc
> +++ b/winsup/cygwin/fhandler_console.cc
> @@ -1794,6 +1794,16 @@ bool fhandler_console::write_console (PWCHAR buf, 
> DWORD len, DWORD& done)
> len -= 4;
>   }
>  }
> +  /* Workaround for ^@ (0x00) handling in xterm compatible mode. */
> +  if (wincap.has_con_24bit_colors () && !con_is_legacy)
> +{
> +  WCHAR *p = buf;
> +  while ((p = wmemchr (p, L'\0', len - (p - buf
> + {
> +   memmove (p, p+1, (len - (p+1 - buf))*sizeof (WCHAR));
> +   len --;
> + }
> +}
>  
>if (con.iso_2022_G1
>   ? con.vt100_graphics_mode_G1
> -- 
> 2.21.0

Counter-proposal:

diff --git a/winsup/cygwin/fhandler_console.cc 
b/winsup/cygwin/fhandler_console.cc
index 66e645aa1774..1b3aa0f34aa6 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -2641,8 +2641,9 @@ fhandler_console::write_normal (const unsigned char *src,
   memset (&ps, 0, sizeof ps);
   while (found < end
 && found - src < CONVERT_LIMIT
+&& base_chars[*found] != IGN
 && ((wincap.has_con_24bit_colors () && !con_is_legacy)
-|| base_chars[*found] == NOR) )
+|| base_chars[*found] == NOR))
 {
   switch (ret = f_mbtowc (_REENT, NULL, (const char *) found,
   end - found, &ps))
@@ -2732,7 +2733,8 @@ do_print:
  cursor_rel (-1, 0);
  break;
case IGN:
- cursor_rel (1, 0);
+if (!wincap.has_con_24bit_colors () || con_is_legacy)
+   cursor_rel (1, 0);
  break;
case CR:
  cursor_get (&x, &y);

But, here's a question: Why do we move the cursor to the right at all?
I assume this is compatible with legacy mode, right?


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer


signature.asc
Description: PGP signature


Re: [PATCH] Cygwin: console: Ignore 0x00 on write().

2020-02-20 Thread Corinna Vinschen
On Feb 20 14:35, Corinna Vinschen wrote:
> On Feb 20 20:51, Takashi Yano wrote:
> > - In xterm compatible mode, 0x00 on write() behaves incompatible
> >   with real xterm. In xterm, 0x00 completely ignored. Therefore,
> >   0x00 is ignored by console with this patch.
> > ---
> >  winsup/cygwin/fhandler_console.cc | 10 ++
> >  1 file changed, 10 insertions(+)
> > [...]
> 
> Counter-proposal:
> 
> diff --git a/winsup/cygwin/fhandler_console.cc 
> b/winsup/cygwin/fhandler_console.cc
> index 66e645aa1774..1b3aa0f34aa6 100644
> --- a/winsup/cygwin/fhandler_console.cc
> +++ b/winsup/cygwin/fhandler_console.cc
> [...]

Btw., I tested this with

  write (1, "A\0B\0C\0D", 7);

it turned out that this results in broken output even with your patch.
The reason is that a NUL byte must not (cannot) be evaluated by 
dev_console::str_to_con() -> sys_cp_mbstowcs().  The latter doesn't
handle embedded NUL bytes gracefully.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer


signature.asc
Description: PGP signature


Re: [PATCH] Cygwin: console: Ignore 0x00 on write().

2020-02-20 Thread Takashi Yano
On Thu, 20 Feb 2020 14:44:59 +0100
Corinna Vinschen wrote:
> On Feb 20 14:35, Corinna Vinschen wrote:
> > On Feb 20 20:51, Takashi Yano wrote:
> > > - In xterm compatible mode, 0x00 on write() behaves incompatible
> > >   with real xterm. In xterm, 0x00 completely ignored. Therefore,
> > >   0x00 is ignored by console with this patch.
> > > ---
> > >  winsup/cygwin/fhandler_console.cc | 10 ++
> > >  1 file changed, 10 insertions(+)
> > > [...]
> > 
> > Counter-proposal:
> > 
> > diff --git a/winsup/cygwin/fhandler_console.cc 
> > b/winsup/cygwin/fhandler_console.cc
> > index 66e645aa1774..1b3aa0f34aa6 100644
> > --- a/winsup/cygwin/fhandler_console.cc
> > +++ b/winsup/cygwin/fhandler_console.cc
> > [...]
> 
> Btw., I tested this with
> 
>   write (1, "A\0B\0C\0D", 7);
> 
> it turned out that this results in broken output even with your patch.
> The reason is that a NUL byte must not (cannot) be evaluated by 
> dev_console::str_to_con() -> sys_cp_mbstowcs().  The latter doesn't
> handle embedded NUL bytes gracefully.

Indeed. Your patch is much better.

On Thu, 20 Feb 2020 14:35:31 +0100
Corinna Vinschen wrote:
> But, here's a question: Why do we move the cursor to the right at all?
> I assume this is compatible with legacy mode, right?

Hmm. This may be a bug of legacy console.
https://en.wikipedia.org/wiki/Null_character
says
(some terminals, however, incorrectly display it as space)

What about ignoring NUL in legacy mode too?

-- 
Takashi Yano 


Re: [PATCH] Cygwin: console: Ignore 0x00 on write().

2020-02-20 Thread Corinna Vinschen
On Feb 20 23:13, Takashi Yano wrote:
> On Thu, 20 Feb 2020 14:44:59 +0100
> Corinna Vinschen wrote:
> > On Feb 20 14:35, Corinna Vinschen wrote:
> > > On Feb 20 20:51, Takashi Yano wrote:
> > > > - In xterm compatible mode, 0x00 on write() behaves incompatible
> > > >   with real xterm. In xterm, 0x00 completely ignored. Therefore,
> > > >   0x00 is ignored by console with this patch.
> > > > ---
> > > >  winsup/cygwin/fhandler_console.cc | 10 ++
> > > >  1 file changed, 10 insertions(+)
> > > > [...]
> > > 
> > > Counter-proposal:
> > > 
> > > diff --git a/winsup/cygwin/fhandler_console.cc 
> > > b/winsup/cygwin/fhandler_console.cc
> > > index 66e645aa1774..1b3aa0f34aa6 100644
> > > --- a/winsup/cygwin/fhandler_console.cc
> > > +++ b/winsup/cygwin/fhandler_console.cc
> > > [...]
> > 
> > Btw., I tested this with
> > 
> >   write (1, "A\0B\0C\0D", 7);
> > 
> > it turned out that this results in broken output even with your patch.
> > The reason is that a NUL byte must not (cannot) be evaluated by 
> > dev_console::str_to_con() -> sys_cp_mbstowcs().  The latter doesn't
> > handle embedded NUL bytes gracefully.
> 
> Indeed. Your patch is much better.
> 
> On Thu, 20 Feb 2020 14:35:31 +0100
> Corinna Vinschen wrote:
> > But, here's a question: Why do we move the cursor to the right at all?
> > I assume this is compatible with legacy mode, right?
> 
> Hmm. This may be a bug of legacy console.
> https://en.wikipedia.org/wiki/Null_character
> says
> (some terminals, however, incorrectly display it as space)
> 
> What about ignoring NUL in legacy mode too?

I'd like that, but this may be a problem in terms of backward
compatibility.  The behaviour is so old, it actually precedes even the
import of Cygwin code into the original CVS repository, 20 years ago...


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer


signature.asc
Description: PGP signature


Re: [PATCH] Cygwin: console: Ignore 0x00 on write().

2020-02-20 Thread Takashi Yano
On Thu, 20 Feb 2020 15:22:45 +0100
Corinna Vinschen wrote:
> On Feb 20 23:13, Takashi Yano wrote:
> > On Thu, 20 Feb 2020 14:44:59 +0100
> > Corinna Vinschen wrote:
> > > On Feb 20 14:35, Corinna Vinschen wrote:
> > > > On Feb 20 20:51, Takashi Yano wrote:
> > > > > - In xterm compatible mode, 0x00 on write() behaves incompatible
> > > > >   with real xterm. In xterm, 0x00 completely ignored. Therefore,
> > > > >   0x00 is ignored by console with this patch.
> > > > > ---
> > > > >  winsup/cygwin/fhandler_console.cc | 10 ++
> > > > >  1 file changed, 10 insertions(+)
> > > > > [...]
> > > > 
> > > > Counter-proposal:
> > > > 
> > > > diff --git a/winsup/cygwin/fhandler_console.cc 
> > > > b/winsup/cygwin/fhandler_console.cc
> > > > index 66e645aa1774..1b3aa0f34aa6 100644
> > > > --- a/winsup/cygwin/fhandler_console.cc
> > > > +++ b/winsup/cygwin/fhandler_console.cc
> > > > [...]
> > > 
> > > Btw., I tested this with
> > > 
> > >   write (1, "A\0B\0C\0D", 7);
> > > 
> > > it turned out that this results in broken output even with your patch.
> > > The reason is that a NUL byte must not (cannot) be evaluated by 
> > > dev_console::str_to_con() -> sys_cp_mbstowcs().  The latter doesn't
> > > handle embedded NUL bytes gracefully.
> > 
> > Indeed. Your patch is much better.
> > 
> > On Thu, 20 Feb 2020 14:35:31 +0100
> > Corinna Vinschen wrote:
> > > But, here's a question: Why do we move the cursor to the right at all?
> > > I assume this is compatible with legacy mode, right?
> > 
> > Hmm. This may be a bug of legacy console.
> > https://en.wikipedia.org/wiki/Null_character
> > says
> > (some terminals, however, incorrectly display it as space)
> > 
> > What about ignoring NUL in legacy mode too?
> 
> I'd like that, but this may be a problem in terms of backward
> compatibility.  The behaviour is so old, it actually precedes even the
> import of Cygwin code into the original CVS repository, 20 years ago...

If so, can't we say it is the *specification* of TERM=cygwin
that NUL moves the cursor right?

-- 
Takashi Yano 


Re: [PATCH] Cygwin: console: Ignore 0x00 on write().

2020-02-20 Thread Corinna Vinschen
On Feb 20 23:49, Takashi Yano wrote:
> On Thu, 20 Feb 2020 15:22:45 +0100
> Corinna Vinschen wrote:
> > On Feb 20 23:13, Takashi Yano wrote:
> > > On Thu, 20 Feb 2020 14:44:59 +0100
> > > Corinna Vinschen wrote:
> > > > On Feb 20 14:35, Corinna Vinschen wrote:
> > > > > On Feb 20 20:51, Takashi Yano wrote:
> > > > > > - In xterm compatible mode, 0x00 on write() behaves incompatible
> > > > > >   with real xterm. In xterm, 0x00 completely ignored. Therefore,
> > > > > >   0x00 is ignored by console with this patch.
> > > > > > ---
> > > > > >  winsup/cygwin/fhandler_console.cc | 10 ++
> > > > > >  1 file changed, 10 insertions(+)
> > > > > > [...]
> > > > > 
> > > > > Counter-proposal:
> > > > > 
> > > > > diff --git a/winsup/cygwin/fhandler_console.cc 
> > > > > b/winsup/cygwin/fhandler_console.cc
> > > > > index 66e645aa1774..1b3aa0f34aa6 100644
> > > > > --- a/winsup/cygwin/fhandler_console.cc
> > > > > +++ b/winsup/cygwin/fhandler_console.cc
> > > > > [...]
> > > > 
> > > > Btw., I tested this with
> > > > 
> > > >   write (1, "A\0B\0C\0D", 7);
> > > > 
> > > > it turned out that this results in broken output even with your patch.
> > > > The reason is that a NUL byte must not (cannot) be evaluated by 
> > > > dev_console::str_to_con() -> sys_cp_mbstowcs().  The latter doesn't
> > > > handle embedded NUL bytes gracefully.
> > > 
> > > Indeed. Your patch is much better.
> > > 
> > > On Thu, 20 Feb 2020 14:35:31 +0100
> > > Corinna Vinschen wrote:
> > > > But, here's a question: Why do we move the cursor to the right at all?
> > > > I assume this is compatible with legacy mode, right?
> > > 
> > > Hmm. This may be a bug of legacy console.
> > > https://en.wikipedia.org/wiki/Null_character
> > > says
> > > (some terminals, however, incorrectly display it as space)
> > > 
> > > What about ignoring NUL in legacy mode too?
> > 
> > I'd like that, but this may be a problem in terms of backward
> > compatibility.  The behaviour is so old, it actually precedes even the
> > import of Cygwin code into the original CVS repository, 20 years ago...
> 
> If so, can't we say it is the *specification* of TERM=cygwin
> that NUL moves the cursor right?

Good point.  Yes, in that case it's "working as designed" and
we just leave it as is.  I push my patch.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer


signature.asc
Description: PGP signature


Re: [PATCH] Cygwin: console: Ignore 0x00 on write().

2020-02-20 Thread Thomas Wolff

On 20.02.2020 17:04, Corinna Vinschen wrote:

On Feb 20 23:49, Takashi Yano wrote:

On Thu, 20 Feb 2020 15:22:45 +0100
Corinna Vinschen wrote:

On Feb 20 23:13, Takashi Yano wrote:

On Thu, 20 Feb 2020 14:44:59 +0100
Corinna Vinschen wrote:

On Feb 20 14:35, Corinna Vinschen wrote:

On Feb 20 20:51, Takashi Yano wrote:

- In xterm compatible mode, 0x00 on write() behaves incompatible
   with real xterm. In xterm, 0x00 completely ignored. Therefore,
   0x00 is ignored by console with this patch.
---
  winsup/cygwin/fhandler_console.cc | 10 ++
  1 file changed, 10 insertions(+)
[...]

Counter-proposal:

diff --git a/winsup/cygwin/fhandler_console.cc 
b/winsup/cygwin/fhandler_console.cc
index 66e645aa1774..1b3aa0f34aa6 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
[...]

Btw., I tested this with

   write (1, "A\0B\0C\0D", 7);

it turned out that this results in broken output even with your patch.
The reason is that a NUL byte must not (cannot) be evaluated by
dev_console::str_to_con() -> sys_cp_mbstowcs().  The latter doesn't
handle embedded NUL bytes gracefully.

Indeed. Your patch is much better.

On Thu, 20 Feb 2020 14:35:31 +0100
Corinna Vinschen wrote:

But, here's a question: Why do we move the cursor to the right at all?
I assume this is compatible with legacy mode, right?

Hmm. This may be a bug of legacy console.
https://en.wikipedia.org/wiki/Null_character
says
(some terminals, however, incorrectly display it as space)

What about ignoring NUL in legacy mode too?

I'd like that, but this may be a problem in terms of backward
compatibility.  The behaviour is so old, it actually precedes even the
import of Cygwin code into the original CVS repository, 20 years ago...

If so, can't we say it is the *specification* of TERM=cygwin
that NUL moves the cursor right?

Good point.  Yes, in that case it's "working as designed" and
we just leave it as is.  I push my patch.
See `man 5 terminfo`: if NUL does anything else than just padding, the 
terminfo entry must contain a pad or npc entry, which it doesn't.
Trouble to be expected. I'd rather suggest to align the design with 
applications' expectations.


Re: [PATCH] Cygwin: console: Ignore 0x00 on write().

2020-02-20 Thread Corinna Vinschen
On Feb 20 17:22, Thomas Wolff wrote:
> On 20.02.2020 17:04, Corinna Vinschen wrote:
> > On Feb 20 23:49, Takashi Yano wrote:
> > > On Thu, 20 Feb 2020 15:22:45 +0100
> > > Corinna Vinschen wrote:
> > > > On Feb 20 23:13, Takashi Yano wrote:
> > > > > On Thu, 20 Feb 2020 14:44:59 +0100
> > > > > Corinna Vinschen wrote:
> > > > > > But, here's a question: Why do we move the cursor to the right at 
> > > > > > all?
> > > > > > I assume this is compatible with legacy mode, right?
> > > > > Hmm. This may be a bug of legacy console.
> > > > > https://en.wikipedia.org/wiki/Null_character
> > > > > says
> > > > > (some terminals, however, incorrectly display it as space)
> > > > > 
> > > > > What about ignoring NUL in legacy mode too?
> > > > I'd like that, but this may be a problem in terms of backward
> > > > compatibility.  The behaviour is so old, it actually precedes even the
> > > > import of Cygwin code into the original CVS repository, 20 years ago...
> > > If so, can't we say it is the *specification* of TERM=cygwin
> > > that NUL moves the cursor right?
> > Good point.  Yes, in that case it's "working as designed" and
> > we just leave it as is.  I push my patch.
> See `man 5 terminfo`: if NUL does anything else than just padding, the
> terminfo entry must contain a pad or npc entry, which it doesn't.
> Trouble to be expected. I'd rather suggest to align the design with
> applications' expectations.

Is that the cygwin terminfo or the xterm terminfo you're talking about?

In case of the cygwin terminfo, that would mean the cygwin terminal
emulation behaves differently from the terminfo for ages.  I guess
you're right then, we should fix this in the cygwin terminal emulation
to make sure it behaves as descibed in its terminfo.

In case of the xterm terminfo, that would be no problem because my patch
drops the cursor movement for NUL.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer


signature.asc
Description: PGP signature


RE: [PATCH] Cygwin: console: Ignore 0x00 on write().

2020-02-20 Thread Lavrentiev, Anton (NIH/NLM/NCBI) [C] via cygwin-patches
JFYI:

Both 0x00 (NUL) and 0x7F (DEL) used to be the filler characters, and were 
ignored by most (hardware) terminals from the very early days.

HTH,
Anton

> -Original Message-
> From: cygwin-patches-ow...@cygwin.com  On
> Behalf Of Takashi Yano
> Sent: Thursday, February 20, 2020 6:52 AM
> To: cygwin-patches@cygwin.com
> Cc: Takashi Yano 
> Subject: [PATCH] Cygwin: console: Ignore 0x00 on write().
> 
> - In xterm compatible mode, 0x00 on write() behaves incompatible
>   with real xterm. In xterm, 0x00 completely ignored. Therefore,
>   0x00 is ignored by console with this patch.
> ---
>  winsup/cygwin/fhandler_console.cc | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/winsup/cygwin/fhandler_console.cc
> b/winsup/cygwin/fhandler_console.cc
> index 66e645aa1..705ce696e 100644
> --- a/winsup/cygwin/fhandler_console.cc
> +++ b/winsup/cygwin/fhandler_console.cc
> @@ -1794,6 +1794,16 @@ bool fhandler_console::write_console (PWCHAR buf,
> DWORD len, DWORD& done)
> len -= 4;
>   }
>  }
> +  /* Workaround for ^@ (0x00) handling in xterm compatible mode. */
> +  if (wincap.has_con_24bit_colors () && !con_is_legacy)
> +{
> +  WCHAR *p = buf;
> +  while ((p = wmemchr (p, L'\0', len - (p - buf
> + {
> +   memmove (p, p+1, (len - (p+1 - buf))*sizeof (WCHAR));
> +   len --;
> + }
> +}
> 
>if (con.iso_2022_G1
>   ? con.vt100_graphics_mode_G1
> --
> 2.21.0