ws2_32: Add support and tests for WSARecvMsg and IP_PKTINFO.

Mike Kaplinskiy mike.kaplinskiy at gmail.com
Sat Oct 16 14:41:59 CDT 2010


On Thu, Oct 14, 2010 at 10:08 PM, Erich Hoover <ehoover at mines.edu> wrote:
> If you guys would mind looking over this revised patchset I would
> greatly appreciate it, I believe I've appropriately included your
> corrections.  Please note that mswsock.h and ws2ipdef.h will be
> submitted as separate patches.  Thanks in advance!
>
> Erich Hoover
> ehoover at mines.edu
>

I still have a few comments on the patch, but all of them are minor.
 - You should have the test in a separate patch as well.

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

>+    /* Loop over all the headers, converting as appropriate */
>+    struct cmsghdr *cmsg_unix = (struct cmsghdr *) CMSG_FIRSTHDR(hdr);
>...
>+    for (ptr = cmsg_win; cmsg_unix != NULL; cmsg_unix = CMSG_NXTHDR(hdr, cmsg_unix))
That's just confusing. Please initialize cmsg_unix in the for loop and
ptr outside the for loop.

>+        /* 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.

>+    if (!msg) return SOCKET_ERROR;
That doesn't seem right to me (no SetLastError?). Is there a test for this?

>+#ifdef IP_PKTINFO
>+        case WS_IP_PKTINFO:
>+#endif
You probably don't need the ifdef. convert_sockopt will just fail.

>+    hdr.Control.buf = pktbuf;
.len = sizeof(pktbuf)?

>+        if (!pWSARecvMsg)
>+        {
>+            skip("WSARecvMsg is unsupported, some tests will be skipped.\n");
>+            return;
>+        }
You probably want win_skip. You also leak some sockets here.

Mike.



More information about the wine-devel mailing list