Re: Bug in Control-d handling?

[Ken Brown <kbrown@cornell.edu>]

[-- 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