Problem with line buffering and getc function on 1.7.33.

Problem with line buffering and getc function on 1.7.33.

[Kaz Kylheku]

Hi all,

On a Cygwin installation version 1.7.33-2(0.280/5/3), I encountered an 
odd issue.

I wrote it up in a StackOverflow question:

http://stackoverflow.com/questions/35928828/cygwin-missing-stream-data-due-to-stdio-putc-line-buffering

This contains a repro test case.

For the benefit of non-users of WWW, the summary is this:
I'm sending data to a stdio stream which has been fdopen-ed from a 
socket
descriptor (which could be irrelevant). The stream is put into line
buffered mode with setvbuf.  When the getc function is used to perform
output on this stream, it loses data. When the string "REPLY\n" is sent,
character by character, the receiver receives only "\n".

If line buffering is not employed, or of putc(ch, stream) is replaced
by fputs(str, stream) (where str is a one-character-long string 
containing
the character to be output), then things work as expected.

Here is a patch I put into the real program where this issue was
found:

http://www.kylheku.com/cgit/txr/commit/?id=3c5f110cc29f8d3fbf2069c68d25ccebee46679e

with the patch, TXR's socket test cases pass on Cygwin.
The tests pass on Solaris 10, various GNU/Linuxes and Mac OS 10.7.3.




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

Re: Problem with line buffering and getc function on 1.7.33.

[Yaakov Selkowitz]

On 2016-03-11 13:16, Kaz Kylheku wrote:
> On a Cygwin installation version 1.7.33-2(0.280/5/3), I encountered an
> odd issue.

This is a 15-month old release which is no longer supported.  Please 
update to 2.4.1.

-- 
Yaakov

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

Re: Problem with line buffering and getc function on 1.7.33.

[Corinna Vinschen]

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

On Mar 13 12:06, Corinna Vinschen wrote:
> On Mar 12 16:41, Kaz Kylheku wrote:
> > On 12.03.2016 14:29, Corinna Vinschen wrote:
> > >I do now.  Basically it's setvbuf screwing up the internal flags in the
> > >FILE structure.  I took the liberty to update newlib's setvbuf to the
> > >OpenBSD version locally and I'm going to apply my patches to newlib
> > >soon.  I'll provide a new 2.5.0 test release of Cygwin with this patch
> > >tomorrow or early next week.
> > 
> > The change in git now seems risky; it substantially rewrites setvbuf.
> > Of course, it's not that I think OpenBSD has it wrong, but that it's
> > being cherry-picked in isolation into what looks like a code base
> > with some other old pieces. Just a thought.
> 
> Point taken.  I compared the code carefully and I'm reasonable sure
> that the risk is low.  The major differences to the old setvbuf are:
> 
> - The locking call is later.  The first check potenitally exiting
>   the function early does not need any access to either reent or fp.
> 
> - OpenBSD setvbuf now drops the ungetc buffer.
> 
> - OpenBSD resets more flags, namely __SOPT | __SNPT | __SEOF.  That's
>   certainly the safer option.
> 
> - Optimal IO size handling is a bonus.  Just setting buffer to 1K was
>   a bit sub-optimal.
> 
> - Add missing __sinit() call.
> 
> - Only set buffersize to non-0 in _IOLBF case if we're already writing.
> 
> - The rest is equivalent to before.  Only the switch statement has been
>   changed to an if in OpenBSD's setvbuf.
> 
> Did you try Cygwin 2.0.5-0.6?

s/2.0.5/2.5.0/


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Problem with line buffering and getc function on 1.7.33.

[Corinna Vinschen]

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

On Mar 12 16:41, Kaz Kylheku wrote:
> On 12.03.2016 14:29, Corinna Vinschen wrote:
> >I do now.  Basically it's setvbuf screwing up the internal flags in the
> >FILE structure.  I took the liberty to update newlib's setvbuf to the
> >OpenBSD version locally and I'm going to apply my patches to newlib
> >soon.  I'll provide a new 2.5.0 test release of Cygwin with this patch
> >tomorrow or early next week.
> 
> The change in git now seems risky; it substantially rewrites setvbuf.
> Of course, it's not that I think OpenBSD has it wrong, but that it's
> being cherry-picked in isolation into what looks like a code base
> with some other old pieces. Just a thought.

Point taken.  I compared the code carefully and I'm reasonable sure
that the risk is low.  The major differences to the old setvbuf are:

- The locking call is later.  The first check potenitally exiting
  the function early does not need any access to either reent or fp.

- OpenBSD setvbuf now drops the ungetc buffer.

- OpenBSD resets more flags, namely __SOPT | __SNPT | __SEOF.  That's
  certainly the safer option.

- Optimal IO size handling is a bonus.  Just setting buffer to 1K was
  a bit sub-optimal.

- Add missing __sinit() call.

- Only set buffersize to non-0 in _IOLBF case if we're already writing.

- The rest is equivalent to before.  Only the switch statement has been
  changed to an if in OpenBSD's setvbuf.

Did you try Cygwin 2.0.5-0.6?


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Problem with line buffering and getc function on 1.7.33.

[Corinna Vinschen]

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

On Mar 12 16:37, Kaz Kylheku wrote:
> On 12.03.2016 14:29, Corinna Vinschen wrote:
> >On Mar 12 20:39, Corinna Vinschen wrote:
> >>On Mar 11 16:05, Kaz Kylheku wrote:
> >>> We can reproduce the problem with just file streams using
> >>> a much simpler program:
> >>> [...]
> >>> The contents of file end up being "\n": one empty
> >>> line, instead of "abcde\n":
> >>>
> >>>   $ cat file
> >>>
> >>>   $
> >>
> >>Thanks for the testcase.  I can reproduce the issue and I see where the
> >>problem occurs, but I'm still puzzled.  Comparing the code in our newlib
> >>C library with its BSD counterparts, I could swear the same behaviour
> >>happens on OpenBSD as well.  If not (which needs testing), I wonder why
> >>and where newlib's actually different.  Right now I don't see the
> >>difference.
> >
> >I do now.  Basically it's setvbuf screwing up the internal flags in the
> >FILE structure.  I took the liberty to update newlib's setvbuf to the
> >OpenBSD version locally and I'm going to apply my patches to newlib
> >soon.  I'll provide a new 2.5.0 test release of Cygwin with this patch
> >tomorrow or early next week.
> 
> 
> Indeed, I thought it would have something to do with the
> stream->_flags, because when I single-stepped over the getc(stream)
> line (necessary for the problem to repro, IIRC) the only apparent state
> change in the stream was to the _flags member:

The actual problem was that setvbuf set the _lbfsize member to -bufsize
in the _IOLBF case indiscriminately.  This in turn leads to __sputc_r
doing the wrong thing:

  if (--_p->_w >= 0 || (_p->_w >= _p->_lbfsize && (char)_c != '\n'))
    return (*_p->_p++ = _c);
  else
    return (__swbuf_r(_ptr, _c, _p));

Note the check for _p->_w >= _p->_lbfsize.  By setting _lbfsize in
setvbuf, what happens is that the first branch is called for all
chars up to the first \n.  However, the *first* call to __sputc_r
is supposed to call __swbuf_r anyway, otherwise bookkeeping is broken.

And that's the major difference to the OpenBSD setvbuf; it only sets
fp->_lbfsize to -bufsize if we're already writing (__SWR flag is set).


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Problem with line buffering and getc function on 1.7.33.

[Kaz Kylheku]

On 12.03.2016 14:29, Corinna Vinschen wrote:
> I do now.  Basically it's setvbuf screwing up the internal flags in the
> FILE structure.  I took the liberty to update newlib's setvbuf to the
> OpenBSD version locally and I'm going to apply my patches to newlib
> soon.  I'll provide a new 2.5.0 test release of Cygwin with this patch
> tomorrow or early next week.

The change in git now seems risky; it substantially rewrites setvbuf.
Of course, it's not that I think OpenBSD has it wrong, but that it's
being cherry-picked in isolation into what looks like a code base
with some other old pieces. Just a thought.





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

Re: Problem with line buffering and getc function on 1.7.33.

[Kaz Kylheku]

On 12.03.2016 14:29, Corinna Vinschen wrote:
> On Mar 12 20:39, Corinna Vinschen wrote:
>> On Mar 11 16:05, Kaz Kylheku wrote:
>> > We can reproduce the problem with just file streams using
>> > a much simpler program:
>> >
>> > #include <stdio.h>
>> >
>> > int main(void)
>> > {
>> >   FILE *out = fopen("file", "w+");
>> >   setvbuf(out, (char *) NULL, _IOLBF, 0);
>> >   getc(out);
>> >   clearerr(out);
>> >   fseek(out, 0, SEEK_SET);
>> >   putc('a', out);
>> >   putc('b', out);
>> >   putc('c', out);
>> >   putc('d', out);
>> >   putc('e', out);
>> >   putc('\n', out);
>> >   fclose(out);
>> >   return 0;
>> > }
>> >
>> > The contents of file end up being "\n": one empty
>> > line, instead of "abcde\n":
>> >
>> >   $ cat file
>> >
>> >   $
>> 
>> Thanks for the testcase.  I can reproduce the issue and I see where 
>> the
>> problem occurs, but I'm still puzzled.  Comparing the code in our 
>> newlib
>> C library with its BSD counterparts, I could swear the same behaviour
>> happens on OpenBSD as well.  If not (which needs testing), I wonder 
>> why
>> and where newlib's actually different.  Right now I don't see the
>> difference.
> 
> I do now.  Basically it's setvbuf screwing up the internal flags in the
> FILE structure.  I took the liberty to update newlib's setvbuf to the
> OpenBSD version locally and I'm going to apply my patches to newlib
> soon.  I'll provide a new 2.5.0 test release of Cygwin with this patch
> tomorrow or early next week.


Indeed, I thought it would have something to do with the
stream->_flags, because when I single-stepped over the getc(stream)
line (necessary for the problem to repro, IIRC) the only apparent state
change in the stream was to the _flags member:

(gdb) n
8         getc(out);
(gdb) p *out
$2 = {_p = 0x80039c28 "", _r = 0, _w = 0, _flags = -30575, _file = 3, 
_bf = {
                                           ^^^^^^^^^^^^^^^^

     _base = 0x80039c28 "", _size = 1024}, _lbfsize = -1024, _data = 0x0,
   _cookie = 0x80039a30, _read = 0x61164cb0 <__sread>,
   _write = 0x61164f30 <__swrite64>, _seek = 0x61164e50 <__sseek>,
   _close = 0x61164ea0 <__sclose>, _ub = {_base = 0x0, _size = 0}, _up = 
0x0,
   _ur = 0, _ubuf = "\000\000", _nbuf = "", _lb = {_base = 0x0, _size = 
0},
   _blksize = 0, _flags2 = 0, _offset = 0, _seek64 = 0x61164ed0 
<__sseek64>,
   _lock = 0x80039bf8, _mbstate = {__count = 0, __value = {__wch = 0,
       __wchb = "\000\000\000"}}}
(gdb) n
9         clearerr(out);
(gdb) p *out
$3 = {_p = 0x80039c28 "", _r = 0, _w = 0, _flags = -22347, _file = 3, 
_bf = {
                                          ^^^^^^^^^^^^^^^^^^

     _base = 0x80039c28 "", _size = 1024}, _lbfsize = -1024, _data = 0x0,
   _cookie = 0x80039a30, _read = 0x61164cb0 <__sread>,
   _write = 0x61164f30 <__swrite64>, _seek = 0x61164e50 <__sseek>,
   _close = 0x61164ea0 <__sclose>, _ub = {_base = 0x0, _size = 0}, _up = 
0x0,
   _ur = 0, _ubuf = "\000\000", _nbuf = "", _lb = {_base = 0x0, _size = 
0},
   _blksize = 0, _flags2 = 0, _offset = 0, _seek64 = 0x61164ed0 
<__sseek64>,
   _lock = 0x80039bf8, _mbstate = {__count = 0, __value = {__wch = 0,
       __wchb = "\000\000\000"}}}



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

Re: Problem with line buffering and getc function on 1.7.33.

[Corinna Vinschen]

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

On Mar 12 20:39, Corinna Vinschen wrote:
> On Mar 11 16:05, Kaz Kylheku wrote:
> > We can reproduce the problem with just file streams using
> > a much simpler program:
> > 
> > #include <stdio.h>
> > 
> > int main(void)
> > {
> >   FILE *out = fopen("file", "w+");
> >   setvbuf(out, (char *) NULL, _IOLBF, 0);
> >   getc(out);
> >   clearerr(out);
> >   fseek(out, 0, SEEK_SET);
> >   putc('a', out);
> >   putc('b', out);
> >   putc('c', out);
> >   putc('d', out);
> >   putc('e', out);
> >   putc('\n', out);
> >   fclose(out);
> >   return 0;
> > }
> > 
> > The contents of file end up being "\n": one empty
> > line, instead of "abcde\n":
> > 
> >   $ cat file
> > 
> >   $
> 
> Thanks for the testcase.  I can reproduce the issue and I see where the
> problem occurs, but I'm still puzzled.  Comparing the code in our newlib
> C library with its BSD counterparts, I could swear the same behaviour
> happens on OpenBSD as well.  If not (which needs testing), I wonder why
> and where newlib's actually different.  Right now I don't see the
> difference.

I do now.  Basically it's setvbuf screwing up the internal flags in the
FILE structure.  I took the liberty to update newlib's setvbuf to the
OpenBSD version locally and I'm going to apply my patches to newlib
soon.  I'll provide a new 2.5.0 test release of Cygwin with this patch
tomorrow or early next week.


Thanks again for the testcase,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Problem with line buffering and getc function on 1.7.33.

[Corinna Vinschen]

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

On Mar 11 16:05, Kaz Kylheku wrote:
> We can reproduce the problem with just file streams using
> a much simpler program:
> 
> #include <stdio.h>
> 
> int main(void)
> {
>   FILE *out = fopen("file", "w+");
>   setvbuf(out, (char *) NULL, _IOLBF, 0);
>   getc(out);
>   clearerr(out);
>   fseek(out, 0, SEEK_SET);
>   putc('a', out);
>   putc('b', out);
>   putc('c', out);
>   putc('d', out);
>   putc('e', out);
>   putc('\n', out);
>   fclose(out);
>   return 0;
> }
> 
> The contents of file end up being "\n": one empty
> line, instead of "abcde\n":
> 
>   $ cat file
> 
>   $

Thanks for the testcase.  I can reproduce the issue and I see where the
problem occurs, but I'm still puzzled.  Comparing the code in our newlib
C library with its BSD counterparts, I could swear the same behaviour
happens on OpenBSD as well.  If not (which needs testing), I wonder why
and where newlib's actually different.  Right now I don't see the
difference.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Problem with line buffering and getc function on 1.7.33.

[Kaz Kylheku]

On 11.03.2016 14:14, Marco Atzeri wrote:
> On 11/03/2016 22:57, Kaz Kylheku wrote:
>> On 11.03.2016 12:08, Yaakov Selkowitz wrote:
>>> On 2016-03-11 13:16, Kaz Kylheku wrote:
>>> 
>>>> On a Cygwin installation version 1.7.33-2(0.280/5/3), I encountered
>>>> an odd issue.
>>> 
>>> This is a 15-month old release which is no longer supported. Please
>>> update to 2.4.1.
>> 
>> Do you mean, "Please update to 2.4.1, it is fixed there!"
>> or do you mean "You haven't done enough with that minimal
>> C repro case; please keep investigating against
>> the latest code?"
> 
> #2 please verify if the issue is still present in the last code.
> We don't have the bandwith to check claims on older release.
> on 2.4.1 the output is:
> 
> $ ./test_stream.exe
> received REQUEST
>  from client
> received
>  from server

That confirms that the issue is still there.

We can reproduce the problem with just file streams using
a much simpler program:

#include <stdio.h>

int main(void)
{
   FILE *out = fopen("file", "w+");
   setvbuf(out, (char *) NULL, _IOLBF, 0);
   getc(out);
   clearerr(out);
   fseek(out, 0, SEEK_SET);
   putc('a', out);
   putc('b', out);
   putc('c', out);
   putc('d', out);
   putc('e', out);
   putc('\n', out);
   fclose(out);
   return 0;
}

The contents of file end up being "\n": one empty
line, instead of "abcde\n":

   $ cat file

   $

The necessary ingredients seem to be: open the stream for update;
put into line buffered mode; do some input. In this program,
replacing putc with fputs doesn't make the problem go away;
the workaround isn't working:

   fputs("a", out);
   fputs("b", out);
   fputs("c", out);
   fputs("d", out);
   fputs("e", out);
   fputs("\n", out);

However, if we do it as one string, then the file does contain
the correct line:

   fputs("abcde\n", out);

This sequence also gets us correct file contents:

   fputs("ab", out);
   putc('c', out);
   putc('d', out);
   putc('e', out);
   putc('\n', out);

HOWEVER, these distracting effects are because GCC is
optimizing one-character fputs calls into _fputc calls.
Annoyingly, this happens even with gcc -O0.

The clearerr and fseek calls are red herrings; they seem to make
make no difference. I put in the fseek for formal ISO C compliance:
ISO C says that before performing an output operation on an update
stream whose last operation was input, we "shall" perform a
positioning operation. (No sanely implemented libc actually
breaks code that doesn't meet this silly requirement.)





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

Re: Problem with line buffering and getc function on 1.7.33.

[Marco Atzeri]

On 11/03/2016 22:57, Kaz Kylheku wrote:
> On 11.03.2016 12:08, Yaakov Selkowitz wrote:
>> On 2016-03-11 13:16, Kaz Kylheku wrote:
>>
>>> On a Cygwin installation version 1.7.33-2(0.280/5/3), I encountered
>>> an odd issue.
>>
>> This is a 15-month old release which is no longer supported. Please
>> update to 2.4.1.
>
> Do you mean, "Please update to 2.4.1, it is fixed there!"
> or do you mean "You haven't done enough with that minimal
> C repro case; please keep investigating against
> the latest code?"

#2 please verify if the issue is still present in the last code.
We don't have the bandwith to check claims on older release.

on 2.4.1 the output is:

$ ./test_stream.exe
received REQUEST
  from client
received
  from server


> There is no benefit to me in reporting this issue
> at all, since the workaround is acceptable and will
> likely keep working indefinitely.

the scope should be to benefit others with the same problem...

Regards
Marco



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

Re: Problem with line buffering and getc function on 1.7.33.

[Kaz Kylheku]

On 11.03.2016 12:08, Yaakov Selkowitz wrote:
> On 2016-03-11 13:16, Kaz Kylheku wrote:
> 
>> On a Cygwin installation version 1.7.33-2(0.280/5/3), I encountered an 
>> odd issue.
> 
> This is a 15-month old release which is no longer supported. Please
> update to 2.4.1.

Do you mean, "Please update to 2.4.1, it is fixed there!"
or do you mean "You haven't done enough with that minimal
C repro case; please keep investigating against
the latest code?"

I'm not looking for support for old versions of Cygwin.
My program *is* supported for users who have that
15-month-old version, and I can provide that support
entirely by myself, as you can see.

There is no benefit to me in reporting this issue
at all, since the workaround is acceptable and will
likely keep working indefinitely.



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