On Thu, Oct 14, 2010 at 10:08 PM, Erich Hoover <ehoo...@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 > ehoo...@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.