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