diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index e2dbf3d3f..b962d7f91 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,11 @@ +2012-05-03 Christopher Faylor + + * DevNotes: Add entry cgf-000002. + * fhandler_tty.cc (bytes_available): Revert to previous Oct-2011 + behavior where a dummy buffer is used to determine how many bytes will + be read. + (fhandler_pty_master::ioctl): Correct coercion in assignment. + 2012-05-03 Corinna Vinschen * net.cc (get_adapters_addresses): Only create thread on affected diff --git a/winsup/cygwin/DevNotes b/winsup/cygwin/DevNotes index 3ca2c0468..a40161140 100644 --- a/winsup/cygwin/DevNotes +++ b/winsup/cygwin/DevNotes @@ -1,3 +1,42 @@ +2012-05-03 cgf-000002 + +<1.7.15> +Fix problem where too much input was attempted to be read from a +pty slave. Fixes: http://cygwin.com/ml/cygwin/2012-05/msg00049.html + + +My change on 2012/04/05 reintroduced the problem first described by: +http://cygwin.com/ml/cygwin/2011-10/threads.html#00445 + +The problem then was, IIRC, due to the fact that bytes sent to the pty +pipe were not written as records. Changing pipe to PIPE_TYPE_MESSAGE in +pipe.cc fixed the problem since writing lines to one side of the pipe +caused exactly that the number of characters to be read on the other +even if there were more characters in the pipe. + +To debug this, I first replaced fhandler_tty.cc with the 1.258, +2012/04/05 version. The test case started working when I did that. + +So, then, I replaced individual functions, one at a time, in +fhandler_tty.cc with their previous versions. I'd expected this to be a +problem with fhandler_pty_master::process_slave_output since that had +seen the most changes but was surprised to see that the culprit was +fhandler_pty_slave::read(). + +The reason was that I really needed the bytes_available() function to +return the number of bytes which would be read in the next operation +rather than the number of bytes available in the pipe. That's because +there may be a number of lines available to be read but the number of +bytes which will be read by ReadFile should reflect the mode of the pty +and, if there is a line to read, only the number of bytes in the line +should be seen as available for the next read. + +Having bytes_available() return the number of bytes which would be read +seemed to fix the problem but it could subtly change the behavior of +other callers of this function. However, I actually think this is +probably a good thing since they probably should have been seeing the +line behavior. + 2012-05-02 cgf-000001 <1.7.15> diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc index 1145bd14f..1428a064e 100644 --- a/winsup/cygwin/fhandler_tty.cc +++ b/winsup/cygwin/fhandler_tty.cc @@ -53,7 +53,11 @@ fhandler_pty_slave::get_unit () bool bytes_available (DWORD& n, HANDLE h) { - bool succeeded = PeekNamedPipe (h, NULL, 0, NULL, &n, NULL); + char buf[INP_BUFFER_SIZE]; + /* Apparently need to pass in a dummy buffer to read a real "record" from + the pipe. So buf is used and then discarded just so we can see how many + bytes will be read by the next ReadFile(). */ + bool succeeded = PeekNamedPipe (h, buf, sizeof (buf), &n, NULL, NULL); if (!succeeded) { termios_printf ("PeekNamedPipe(%p) failed, %E", h); @@ -1429,7 +1433,7 @@ fhandler_pty_master::ioctl (unsigned int cmd, void *arg) set_errno (EINVAL); return -1; } - *(int *) arg = (DWORD) n; + *(int *) arg = (int) n; } break; default: