ws2_32: Add support and tests for WSARecvMsg and IP_PKTINFO.

Mike Kaplinskiy mike.kaplinskiy at gmail.com
Mon Oct 18 21:58:35 CDT 2010


On Sun, Oct 17, 2010 at 5:51 PM, Erich Hoover <ehoover at mines.edu> wrote:
> On Sat, Oct 16, 2010 at 1:41 PM, Mike Kaplinskiy
> <mike.kaplinskiy at gmail.com> wrote:
>> On Thu, Oct 14, 2010 at 10:08 PM, Erich Hoover <ehoover at mines.edu> wrote:
>>> ...
>>>     DWORD                               flags;
>>>     unsigned int                        n_iovecs;
>>>     unsigned int                        first_iovec;
>>>+    WSABUF                             *control;
>>>     struct iovec                        iovec[1];
>>> } ws2_async;
>> I would prefer control declared above n_iovecs purely for
>> organizational purposes (params first, buffers and info about them
>> last). Also you can't just store the passed in WSABUF pointer; it may
>> be stack allocated and may disappear before the overlapped call
>> completes. You could just add control and control_len instead of using
>> WSABUF.
>
> That's why in the original version I had those parameters split out;
> however, I realized that since the length parameter gets modified by
> WSARecvMsg that it is impossible for the WSAMSG structure (or any of
> its parameters) to disappear before WSARecvMsg completes.  I have
> re-written the tests (see patch 4 at the end) to show that this
> structure gets modified even when an overlapped request is used and
> results in modification of the structure at a later time, the results
> from the test bot can be found here:
>    https://testbot.winehq.org/JobDetails.pl?Key=6250
>
Fair enough. Would you mind adding a test to see if the flags are
changed as well (and see if the flags returned by
WSAGetOverlappedResult are of that sort as well)? If it doesn't you
can drop lpFlags and just copy it from flags. Otherwise you may want
to rename it to be less windows-y (like local_flags and flags).

>>>+        /* Insufficient room for control headers */
>>>+        errno = EINVAL;
>> This should probably be ENOMEM then. On that note, it would be nice to
>> test what windows actually does when you don't give it enough space
>> (like 1 byte). We should probably rewrite that function at some point
>> since I'm not sure what errno it returns anymore.
>
> This is odd, I must have copy-pasted the wrong error code from the
> recvmsg() manual page.  I've investigated this issue more and
> discovered that the code should actually be EMSGSIZE.  This change and
> the corresponding tests can be found in my revised patches (at the
> end), I've also included support for reporting back the MSG_CTRUNC
> flag.
>
Hmm, you may also want to warn/err on EMSGSIZE coming from recvmsg.

>>>+#ifdef IP_PKTINFO
>>>+        case WS_IP_PKTINFO:
>>>+#endif
>> You probably don't need the ifdef. convert_sockopt will just fail.
>
> I did this to match how IP_HDRINCL was done nearby, should this really
> be changed?
>
We seem to mix both styles. I don't have any preference then.

Mike.



More information about the wine-devel mailing list