[PATCH 0/1] Fix MSG_WAITALL support

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/1] Fix MSG_WAITALL support

cygwin-patches mailing list
It looks to me like there's been a bug in the MSG_WAITALL support for
AF_INET and AF_LOCAL sockets ever since that support was first
introduced 13 years ago in commit 023a2fa7.  If I'm right, MSG_WAITALL
has never worked.

This patch fixes it.  I'll push it in a few days if no one sees
anything wrong with it.

In a followup email I'll show how I tested it.

Ken Brown (1):
  Cygwin: AF_INET and AF_LOCAL: recv_internal: fix MSG_WAITALL support

 winsup/cygwin/fhandler_socket_inet.cc  | 2 +-
 winsup/cygwin/fhandler_socket_local.cc | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--
2.28.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/1] Cygwin: AF_INET and AF_LOCAL: recv_internal: fix MSG_WAITALL support

cygwin-patches mailing list
If MSG_WAITALL is set, recv_internal calls WSARecv or WSARecvFrom in a
loop, in an effort to fill all the scatter-gather buffers.  The test
for whether all the buffers are full was previously incorrect.
---
 winsup/cygwin/fhandler_socket_inet.cc  | 2 +-
 winsup/cygwin/fhandler_socket_local.cc | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/fhandler_socket_inet.cc b/winsup/cygwin/fhandler_socket_inet.cc
index 71e92e341..bc08d3cf1 100644
--- a/winsup/cygwin/fhandler_socket_inet.cc
+++ b/winsup/cygwin/fhandler_socket_inet.cc
@@ -1208,7 +1208,7 @@ fhandler_socket_inet::recv_internal (LPWSAMSG wsamsg, bool use_recvmsg)
   --wsacnt;
  }
     }
-  if (!wret)
+  if (!wsacnt)
     break;
  }
       else if (WSAGetLastError () != WSAEWOULDBLOCK)
diff --git a/winsup/cygwin/fhandler_socket_local.cc b/winsup/cygwin/fhandler_socket_local.cc
index 8bfba225a..c94bf828f 100644
--- a/winsup/cygwin/fhandler_socket_local.cc
+++ b/winsup/cygwin/fhandler_socket_local.cc
@@ -1212,7 +1212,7 @@ fhandler_socket_local::recv_internal (LPWSAMSG wsamsg, bool use_recvmsg)
   --wsacnt;
  }
     }
-  if (!wret)
+  if (!wsacnt)
     break;
  }
       else if (WSAGetLastError () != WSAEWOULDBLOCK)
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/1] Fix MSG_WAITALL support

cygwin-patches mailing list
In reply to this post by cygwin-patches mailing list
On 10/12/2020 2:02 PM, Ken Brown via Cygwin-patches wrote:
> It looks to me like there's been a bug in the MSG_WAITALL support for
> AF_INET and AF_LOCAL sockets ever since that support was first
> introduced 13 years ago in commit 023a2fa7.  If I'm right, MSG_WAITALL
> has never worked.
>
> This patch fixes it.  I'll push it in a few days if no one sees
> anything wrong with it.
>
> In a followup email I'll show how I tested it.

Attached are slight variants of the server/client programs from Section 57.2 of
Kerrisk's book, "The Linux Programming Interface".  The only essential
difference is that I've changed the server program to (a) use a small buffer
(size 10 instead of 100) and (b) use 'recv' with the MSG_WAITALL flag instead of
'read'.  The 'recv' call shouldn't return until it reads 10 bytes.

To test, run waitall_sv in one terminal and waitall_cl in a second.  Type
something in the second terminal (followed by RET), and it should be echoed in
the first.  But because of the MSG_WAITALL flag, the echoing shouldn't occur
until 10 bytes have been written.  For example, if I type "abcd<RET>" in the
second terminal and then do it again, I should see the following:

# Terminal 2:
$ ./waitall_cl
abcd
abcd

# Terminal 1:
$ ./waitall_sv
abcd
abcd

Here the echoing in Terminal 1 shouldn't occur until I've typed both "abcd"
lines in Terminal 2.

[Note that there is a newline character after each "abcd", so "abcd<RET>" is 5
bytes long, and the two lines together are 10 bytes long.]

Before I apply my patch, each line typed in Terminal 2 is immediately echoed in
Terminal 1.  After I apply the patch, the echoing doesn't occur until I've typed
both lines.

Ken

waitall_sv.c (1K) Download Attachment
waitall_cl.c (994 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/1] Fix MSG_WAITALL support

cygwin-patches mailing list
In reply to this post by cygwin-patches mailing list
On 10/12/2020 2:02 PM, Ken Brown via Cygwin-patches wrote:
> It looks to me like there's been a bug in the MSG_WAITALL support for
> AF_INET and AF_LOCAL sockets ever since that support was first
> introduced 13 years ago in commit 023a2fa7.  If I'm right, MSG_WAITALL
> has never worked.
>
> This patch fixes it.  I'll push it in a few days if no one sees
> anything wrong with it.
>
> In a followup email I'll show how I tested it.

Hi Corinna,

I know I said I'd push this in a few days, but that was when I thought you'd be
gone for a while longer.

Does the patch look OK?

Ken
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/1] Fix MSG_WAITALL support

Corinna Vinschen-2
On Oct 22 15:26, Ken Brown via Cygwin-patches wrote:

> On 10/12/2020 2:02 PM, Ken Brown via Cygwin-patches wrote:
> > It looks to me like there's been a bug in the MSG_WAITALL support for
> > AF_INET and AF_LOCAL sockets ever since that support was first
> > introduced 13 years ago in commit 023a2fa7.  If I'm right, MSG_WAITALL
> > has never worked.
> >
> > This patch fixes it.  I'll push it in a few days if no one sees
> > anything wrong with it.
> >
> > In a followup email I'll show how I tested it.
>
> Hi Corinna,
>
> I know I said I'd push this in a few days, but that was when I thought you'd
> be gone for a while longer.
>
> Does the patch look OK?

Sure!  I mean, you tested it and it fixing a problem, right?


Thanks,
Corinna