Bug in Control-d handling?

Bug in Control-d handling?

[Ken Brown]

Consider the following program, which reads from standard input a line at a time 
and then echoes the input back to the terminal:

$ cat cat_line.c
#include <stdio.h>

int main ()
{
   char buf[BUFSIZ];

   while (fgets (buf, BUFSIZ, stdin))
     fputs (buf, stdout);
}

Run the program, type one or more characters (without hitting Enter), and type 
Ctrl-d until the program exits.  What I expect is that nothing visible happens 
on the first Ctrl-d [but the input is sent to the internal stdin buffer], and 
that the input is echoed and the program exits after the second Ctrl-d [the 
program sees EOF].  This is what happens on Linux.  On Cygwin, however, the 
program keeps running after the second Ctrl-d and doesn't exit until Ctrl-d is 
pressed a third time.

I observed this problem because of a failing Emacs test, in which the program 
"rev" was not seeing EOF after being sent Ctrl-d; "rev" does something like the 
test case above, but using fgetws instead of fgets.

Ken

Re: Bug in Control-d handling?

[Takashi Yano]

On Sun, 19 Jun 2022 15:08:19 -0400
Ken Brown wrote:
> Consider the following program, which reads from standard input a line at a time 
> and then echoes the input back to the terminal:
> 
> $ cat cat_line.c
> #include <stdio.h>
> 
> int main ()
> {
>    char buf[BUFSIZ];
> 
>    while (fgets (buf, BUFSIZ, stdin))
>      fputs (buf, stdout);
> }
> 
> Run the program, type one or more characters (without hitting Enter), and type 
> Ctrl-d until the program exits.  What I expect is that nothing visible happens 
> on the first Ctrl-d [but the input is sent to the internal stdin buffer], and 
> that the input is echoed and the program exits after the second Ctrl-d [the 
> program sees EOF].  This is what happens on Linux.  On Cygwin, however, the 
> program keeps running after the second Ctrl-d and doesn't exit until Ctrl-d is 
> pressed a third time.
> 
> I observed this problem because of a failing Emacs test, in which the program 
> "rev" was not seeing EOF after being sent Ctrl-d; "rev" does something like the 
> test case above, but using fgetws instead of fgets.

Isn't this a bug of newlib? Try following code.

#include <stdio.h>

int main()
{
	printf("%d\n", getchar());
	printf("%d\n", feof(stdin));
	printf("%d\n", getchar());
	return 0;
}

If you press Ctrl-D at the first getchar(), the second getchar()
does not return EOF while it does in linux.

The following patch seems to resolve the issue.

diff --git a/newlib/libc/stdio/refill.c b/newlib/libc/stdio/refill.c
index ccedc7af7..843163b7e 100644
--- a/newlib/libc/stdio/refill.c
+++ b/newlib/libc/stdio/refill.c
@@ -47,11 +47,9 @@ __srefill_r (struct _reent * ptr,
 
   fp->_r = 0;			/* largely a convenience for callers */
 
-#ifndef __CYGWIN__
   /* SysV does not make this test; take it out for compatibility */
   if (fp->_flags & __SEOF)
     return EOF;
-#endif
 
   /* if not already reading, have to be reading and writing */
   if ((fp->_flags & __SRD) == 0)

However, I am not sure what this #ifndef __CYGWIN__ is for.

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

Re: Bug in Control-d handling?

[Takashi Yano]

On Mon, 20 Jun 2022 17:59:35 +0900
Takashi Yano wrote:
> Isn't this a bug of newlib? Try following code.
> 
> #include <stdio.h>
> 
> int main()
> {
> 	printf("%d\n", getchar());
> 	printf("%d\n", feof(stdin));
> 	printf("%d\n", getchar());
> 	return 0;
> }
> 
> If you press Ctrl-D at the first getchar(), the second getchar()
> does not return EOF while it does in linux.
> 
> The following patch seems to resolve the issue.
> 
> diff --git a/newlib/libc/stdio/refill.c b/newlib/libc/stdio/refill.c
> index ccedc7af7..843163b7e 100644
> --- a/newlib/libc/stdio/refill.c
> +++ b/newlib/libc/stdio/refill.c
> @@ -47,11 +47,9 @@ __srefill_r (struct _reent * ptr,
>  
>    fp->_r = 0;			/* largely a convenience for callers */
>  
> -#ifndef __CYGWIN__
>    /* SysV does not make this test; take it out for compatibility */
>    if (fp->_flags & __SEOF)
>      return EOF;
> -#endif
>  
>    /* if not already reading, have to be reading and writing */
>    if ((fp->_flags & __SRD) == 0)
> 
> However, I am not sure what this #ifndef __CYGWIN__ is for.

Ah, I confirmed that System V (Solaris 11.4) behaves like that.
Does cygwin aim for System V compatibility???

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

Re: Bug in Control-d handling?

[Ken Brown]

[-- Attachment #1: Type: text/plain, Size: 1969 bytes --]

On 6/20/2022 6:22 AM, Takashi Yano wrote:
> On Mon, 20 Jun 2022 17:59:35 +0900
> Takashi Yano wrote:
>> Isn't this a bug of newlib? Try following code.
>>
>> #include <stdio.h>
>>
>> int main()
>> {
>> 	printf("%d\n", getchar());
>> 	printf("%d\n", feof(stdin));
>> 	printf("%d\n", getchar());
>> 	return 0;
>> }
>>
>> If you press Ctrl-D at the first getchar(), the second getchar()
>> does not return EOF while it does in linux.
>>
>> The following patch seems to resolve the issue.
>>
>> diff --git a/newlib/libc/stdio/refill.c b/newlib/libc/stdio/refill.c
>> index ccedc7af7..843163b7e 100644
>> --- a/newlib/libc/stdio/refill.c
>> +++ b/newlib/libc/stdio/refill.c
>> @@ -47,11 +47,9 @@ __srefill_r (struct _reent * ptr,
>>   
>>     fp->_r = 0;			/* largely a convenience for callers */
>>   
>> -#ifndef __CYGWIN__
>>     /* SysV does not make this test; take it out for compatibility */
>>     if (fp->_flags & __SEOF)
>>       return EOF;
>> -#endif
>>   
>>     /* if not already reading, have to be reading and writing */
>>     if ((fp->_flags & __SRD) == 0)
>>
>> However, I am not sure what this #ifndef __CYGWIN__ is for.
> 
> Ah, I confirmed that System V (Solaris 11.4) behaves like that.
> Does cygwin aim for System V compatibility???

Thanks for tracking this down!

I don't recall any situation in which Cygwin preferred System V compatibility 
over Linux compatibility.  I'm attaching the commit (from November 2004) in 
which the #ifndef __CYGWIN__ was introduced.  There's no indication in the 
commit message as to the reason for the change.  I also didn't see anything 
relevant in the cygwin or cygwin-developers mailing lists from November 2004, 
but I might have missed it.

I think that commit should probably be reverted, but we should wait until 
Corinna is available.  Even though the issue is in newlib code, the code only 
affects Cygwin, so there's probably no need to involve the newlib list.  But 
again, that's Corinna's call.

Ken

[-- Attachment #2: 0001-libc-stdio-refill.c-__srefill-Try-again-after-EOF-on.patch --]
[-- Type: text/plain, Size: 1710 bytes --]

From 1f8f7e2d54364bd2993892adffb5c6435f53167d Mon Sep 17 00:00:00 2001
From: Christopher Faylor <me@cgf.cx>
Date: Wed, 17 Nov 2004 17:02:10 +0000
Subject: [PATCH] * libc/stdio/refill.c (__srefill): Try again after EOF on
 Cygwin.  Clear EOF flag if successful.

---
 newlib/ChangeLog           | 5 +++++
 newlib/libc/stdio/refill.c | 8 ++++++++
 2 files changed, 13 insertions(+)

diff --git a/newlib/ChangeLog b/newlib/ChangeLog
index 9983e6ba6..8b8cf4d4c 100644
--- a/newlib/ChangeLog
+++ b/newlib/ChangeLog
@@ -1,3 +1,8 @@
+2004-11-17  Christopher Faylor  <cgf@timesys.com>
+
+	* libc/stdio/refill.c (__srefill): Try again after EOF on Cygwin.  Clear
+	EOF flag if successful.
+
 2004-10-28  Christopher Faylor  <cgf@timesys.com>
 
 	* libc/include/sys/signal.h: Move <signal.h> include to bottom of file
diff --git a/newlib/libc/stdio/refill.c b/newlib/libc/stdio/refill.c
index 74573e8fd..3f0b1a566 100644
--- a/newlib/libc/stdio/refill.c
+++ b/newlib/libc/stdio/refill.c
@@ -45,9 +45,11 @@ _DEFUN(__srefill, (fp),
 
   fp->_r = 0;			/* largely a convenience for callers */
 
+#ifndef __CYGWIN__
   /* SysV does not make this test; take it out for compatibility */
   if (fp->_flags & __SEOF)
     return EOF;
+#endif
 
   /* if not already reading, have to be reading and writing */
   if ((fp->_flags & __SRD) == 0)
@@ -98,7 +100,13 @@ _DEFUN(__srefill, (fp),
   fp->_p = fp->_bf._base;
   fp->_r = (*fp->_read) (fp->_cookie, (char *) fp->_p, fp->_bf._size);
   fp->_flags &= ~__SMOD;	/* buffer contents are again pristine */
+#ifndef __CYGWIN__
   if (fp->_r <= 0)
+#else
+  if (fp->_r > 0)
+    fp->_flags &= ~__SEOF;
+  else
+#endif
     {
       if (fp->_r == 0)
 	fp->_flags |= __SEOF;
-- 
2.36.1

Re: Bug in Control-d handling?

[Eliot Moss]

On 6/20/2022 9:24 AM, Ken Brown wrote:
 > On 6/20/2022 6:22 AM, Takashi Yano wrote:
 >> On Mon, 20 Jun 2022 17:59:35 +0900
 >> Takashi Yano wrote:
 >>> Isn't this a bug of newlib? Try following code.
 >>>
 >>> #include <stdio.h>
 >>>
 >>> int main()
 >>> {
 >>>     printf("%d\n", getchar());
 >>>     printf("%d\n", feof(stdin));
 >>>     printf("%d\n", getchar());
 >>>     return 0;
 >>> }
 >>>
 >>> If you press Ctrl-D at the first getchar(), the second getchar()
 >>> does not return EOF while it does in linux.
 >>>
 >>> The following patch seems to resolve the issue.
 >>>
 >>> diff --git a/newlib/libc/stdio/refill.c b/newlib/libc/stdio/refill.c
 >>> index ccedc7af7..843163b7e 100644
 >>> --- a/newlib/libc/stdio/refill.c
 >>> +++ b/newlib/libc/stdio/refill.c
 >>> @@ -47,11 +47,9 @@ __srefill_r (struct _reent * ptr,
 >>>     fp->_r = 0;            /* largely a convenience for callers */
 >>> -#ifndef __CYGWIN__
 >>>     /* SysV does not make this test; take it out for compatibility */
 >>>     if (fp->_flags & __SEOF)
 >>>       return EOF;
 >>> -#endif
 >>>     /* if not already reading, have to be reading and writing */
 >>>     if ((fp->_flags & __SRD) == 0)
 >>>
 >>> However, I am not sure what this #ifndef __CYGWIN__ is for.
 >>
 >> Ah, I confirmed that System V (Solaris 11.4) behaves like that.
 >> Does cygwin aim for System V compatibility???
 >
 > Thanks for tracking this down!
 >
 > I don't recall any situation in which Cygwin preferred System V compatibility over Linux
 > compatibility.  I'm attaching the commit (from November 2004) in which the #ifndef __CYGWIN__ was
 > introduced.  There's no indication in the commit message as to the reason for the change.  I also
 > didn't see anything relevant in the cygwin or cygwin-developers mailing lists from November 2004,
 > but I might have missed it.
 >
 > I think that commit should probably be reverted, but we should wait until Corinna is available.
 > Even though the issue is in newlib code, the code only affects Cygwin, so there's probably no need
 > to involve the newlib list.  But again, that's Corinna's call.

Good idea to wait for Corinna.  I did a smidge of research, looking through
the Wikipedia page on the POSIX terminal interface.  It suggests that this
part was largely borrowed from System-V, so that may explain what we see in
newlib.  And looking up the POSIX spec, it says this:

"Special character on input, which is recognized if the ICANON flag is
set. When received, all the bytes waiting to be read are immediately passed to
the process without waiting for a <newline>, and the EOF is discarded. Thus,
if there are no bytes waiting (that is, the EOF occurred at the beginning of a
line), a byte count of zero shall be returned from the read(), representing an
end-of-file indication. If ICANON is set, the EOF character shall be discarded
when processed."

This suggests that indeed control-D will need to be typed twice before a
blocking read on a terminal with ICANON will appear to be at eof.  I guess we
can consider getchar to be (almost) like a read of size 1, except where read
would return 0, getchar returns -1.

The quoted text above implies that the second control-D will set the
end-of-file indicator.  So I agree that it looks like the ifndef should be
removed.  And of course, wherever we said control-D, EOF should be implied (it
is normally control-D, but the user can change which character is associated
with the EOF semantics).

Cheers - Eliot

Re: Bug in Control-d handling?

[Achim Gratz]

Takashi Yano writes:
> Ah, I confirmed that System V (Solaris 11.4) behaves like that.
> Does cygwin aim for System V compatibility???

Not that I know of, but even if it ever was: the current guideline is
IIRC that if something is unspecified or implementation dependent, then
Cygwin should adhere to what Linux does if possible.


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

Factory and User Sound Singles for Waldorf Q+, Q and microQ:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

Re: Bug in Control-d handling?

[Ken Brown]

On 6/20/2022 9:53 AM, Eliot Moss wrote:
> Good idea to wait for Corinna.  I did a smidge of research, looking through
> the Wikipedia page on the POSIX terminal interface.  It suggests that this
> part was largely borrowed from System-V, so that may explain what we see in
> newlib.  And looking up the POSIX spec, it says this:
> 
> "Special character on input, which is recognized if the ICANON flag is
> set. When received, all the bytes waiting to be read are immediately passed to
> the process without waiting for a <newline>, and the EOF is discarded. Thus,
> if there are no bytes waiting (that is, the EOF occurred at the beginning of a
> line), a byte count of zero shall be returned from the read(), representing an
> end-of-file indication. If ICANON is set, the EOF character shall be discarded
> when processed."
> 
> This suggests that indeed control-D will need to be typed twice before a
> blocking read on a terminal with ICANON will appear to be at eof.  I guess we
> can consider getchar to be (almost) like a read of size 1, except where read
> would return 0, getchar returns -1.
> 
> The quoted text above implies that the second control-D will set the
> end-of-file indicator.  So I agree that it looks like the ifndef should be
> removed.  And of course, wherever we said control-D, EOF should be implied (it
> is normally control-D, but the user can change which character is associated
> with the EOF semantics).

I would frame the issue slightly differently, since this is not just about 
terminals; the code we're discussing applies to any file.  Suppose EOF has been 
seen in a call to one of the stdio input functions.  Prior to commit 1f8f7e2d, 
any further calls to those functions would immediately return EOF.  Since that 
commit, however, another attempt is made to read, and the EOF indicator for the 
file is turned off if input is found.  In many cases (e.g., an ordinary disk 
file), this second read will still return EOF.  In other cases (e.g., a 
terminal), the second read might block if there's still no input available.

Ken

Re: Bug in Control-d handling?

[Brian Inglis]

On 2022-06-20 04:22, Takashi Yano wrote:
> On Mon, 20 Jun 2022 17:59:35 +0900
> Takashi Yano wrote:
>> Isn't this a bug of newlib? Try following code.
>>
>> #include <stdio.h>
>>
>> int main()
>> {
>> 	printf("%d\n", getchar());
>> 	printf("%d\n", feof(stdin));
>> 	printf("%d\n", getchar());
>> 	return 0;
>> }
>>
>> If you press Ctrl-D at the first getchar(), the second getchar()
>> does not return EOF while it does in linux.
>>
>> The following patch seems to resolve the issue.
>>
>> diff --git a/newlib/libc/stdio/refill.c b/newlib/libc/stdio/refill.c
>> index ccedc7af7..843163b7e 100644
>> --- a/newlib/libc/stdio/refill.c
>> +++ b/newlib/libc/stdio/refill.c
>> @@ -47,11 +47,9 @@ __srefill_r (struct _reent * ptr,
>>   
>>     fp->_r = 0;			/* largely a convenience for callers */
>>   
>> -#ifndef __CYGWIN__
>>     /* SysV does not make this test; take it out for compatibility */
>>     if (fp->_flags & __SEOF)
>>       return EOF;
>> -#endif
>>   
>>     /* if not already reading, have to be reading and writing */
>>     if ((fp->_flags & __SRD) == 0)
>>
>> However, I am not sure what this #ifndef __CYGWIN__ is for.
> 
> Ah, I confirmed that System V (Solaris 11.4) behaves like that.
> Does cygwin aim for System V compatibility???

The emphasis has shifted over the years from Solaris etc. compatibility 
when Sparc desktops were ubiquitous (and I switched from DJGPP to 
Cygwin) to Linux etc. compatibility as PCs displaced RISC boxes.
I think ACLs were the last new feature where Solaris was considered vs 
POSIX.

-- 
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: Bug in Control-d handling?

[Corinna Vinschen]

On Jun 20 09:24, Ken Brown wrote:
> On 6/20/2022 6:22 AM, Takashi Yano wrote:
> > On Mon, 20 Jun 2022 17:59:35 +0900
> > Takashi Yano wrote:
> > > Isn't this a bug of newlib? Try following code.
> > > 
> > > #include <stdio.h>
> > > 
> > > int main()
> > > {
> > > 	printf("%d\n", getchar());
> > > 	printf("%d\n", feof(stdin));
> > > 	printf("%d\n", getchar());
> > > 	return 0;
> > > }
> > > 
> > > If you press Ctrl-D at the first getchar(), the second getchar()
> > > does not return EOF while it does in linux.
> > > 
> > > The following patch seems to resolve the issue.
> > > 
> > > diff --git a/newlib/libc/stdio/refill.c b/newlib/libc/stdio/refill.c
> > > index ccedc7af7..843163b7e 100644
> > > --- a/newlib/libc/stdio/refill.c
> > > +++ b/newlib/libc/stdio/refill.c
> > > @@ -47,11 +47,9 @@ __srefill_r (struct _reent * ptr,
> > >     fp->_r = 0;			/* largely a convenience for callers */
> > > -#ifndef __CYGWIN__
> > >     /* SysV does not make this test; take it out for compatibility */
> > >     if (fp->_flags & __SEOF)
> > >       return EOF;
> > > -#endif
> > >     /* if not already reading, have to be reading and writing */
> > >     if ((fp->_flags & __SRD) == 0)
> > > 
> > > However, I am not sure what this #ifndef __CYGWIN__ is for.
> > 
> > Ah, I confirmed that System V (Solaris 11.4) behaves like that.
> > Does cygwin aim for System V compatibility???
> 
> Thanks for tracking this down!
> 
> I don't recall any situation in which Cygwin preferred System V
> compatibility over Linux compatibility.  I'm attaching the commit (from
> November 2004) in which the #ifndef __CYGWIN__ was introduced.  There's no
> indication in the commit message as to the reason for the change.  I also
> didn't see anything relevant in the cygwin or cygwin-developers mailing
> lists from November 2004, but I might have missed it.
> 
> I think that commit should probably be reverted, but we should wait until
> Corinna is available.  Even though the issue is in newlib code, the code
> only affects Cygwin, so there's probably no need to involve the newlib list.
> But again, that's Corinna's call.

I think reverting this patch makes sense these days.

Thanks for tracking this down, please push this on the master branch.


Thanks,
Corinna




> 
> Ken

> From 1f8f7e2d54364bd2993892adffb5c6435f53167d Mon Sep 17 00:00:00 2001
> From: Christopher Faylor <me@cgf.cx>
> Date: Wed, 17 Nov 2004 17:02:10 +0000
> Subject: [PATCH] * libc/stdio/refill.c (__srefill): Try again after EOF on
>  Cygwin.  Clear EOF flag if successful.
> 
> ---
>  newlib/ChangeLog           | 5 +++++
>  newlib/libc/stdio/refill.c | 8 ++++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/newlib/ChangeLog b/newlib/ChangeLog
> index 9983e6ba6..8b8cf4d4c 100644
> --- a/newlib/ChangeLog
> +++ b/newlib/ChangeLog
> @@ -1,3 +1,8 @@
> +2004-11-17  Christopher Faylor  <cgf@timesys.com>
> +
> +	* libc/stdio/refill.c (__srefill): Try again after EOF on Cygwin.  Clear
> +	EOF flag if successful.
> +
>  2004-10-28  Christopher Faylor  <cgf@timesys.com>
>  
>  	* libc/include/sys/signal.h: Move <signal.h> include to bottom of file
> diff --git a/newlib/libc/stdio/refill.c b/newlib/libc/stdio/refill.c
> index 74573e8fd..3f0b1a566 100644
> --- a/newlib/libc/stdio/refill.c
> +++ b/newlib/libc/stdio/refill.c
> @@ -45,9 +45,11 @@ _DEFUN(__srefill, (fp),
>  
>    fp->_r = 0;			/* largely a convenience for callers */
>  
> +#ifndef __CYGWIN__
>    /* SysV does not make this test; take it out for compatibility */
>    if (fp->_flags & __SEOF)
>      return EOF;
> +#endif
>  
>    /* if not already reading, have to be reading and writing */
>    if ((fp->_flags & __SRD) == 0)
> @@ -98,7 +100,13 @@ _DEFUN(__srefill, (fp),
>    fp->_p = fp->_bf._base;
>    fp->_r = (*fp->_read) (fp->_cookie, (char *) fp->_p, fp->_bf._size);
>    fp->_flags &= ~__SMOD;	/* buffer contents are again pristine */
> +#ifndef __CYGWIN__
>    if (fp->_r <= 0)
> +#else
> +  if (fp->_r > 0)
> +    fp->_flags &= ~__SEOF;
> +  else
> +#endif
>      {
>        if (fp->_r == 0)
>  	fp->_flags |= __SEOF;
> -- 
> 2.36.1
> 

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