ws2_32: WSACleanup cleans up open sockets (OSX only)

[Ken Thomases]

Hi,

Thanks for contributing to Wine.  I've commented on the patch in-line below:

On Jul 31, 2015, at 10:19 PM, Matt Durgavich <mattdurgavich at gmail.com> wrote:

> +static void close_open_sockets() {
> +#ifdef HAVE_LIBPROC_H
> +    int pid = getpid();
> +    int bufferSizeNeeded = proc_pidinfo(pid, PROC_PIDLISTFDS, 0, NULL, 0);

This function isn't documented and the header which declares it says it's SPI rather than API and can change at any time.

Also, is this the right approach generally?  This is going to close all Unix sockets opened by the process, not just Win32 sockets opened by ws2_32.  Is it safe/proper for WSACleanup to close sockets that it didn't open?  Isn't it possible that other parts of Wine could open Unix sockets for other reasons?  Or that system libraries or frameworks might do so to carry out requests made by Wine?

> +    if (bufferSizeNeeded > 0) {
> +        struct proc_fdinfo *infos = (struct proc_fdinfo*)malloc(bufferSizeNeeded);
> +        int ret = proc_pidinfo(pid, PROC_PIDLISTFDS, 0, infos, bufferSizeNeeded);
> +        int numfds = bufferSizeNeeded / PROC_PIDLISTFD_SIZE;

Shouldn't you use ret / PROC_PIDLISTFD_SIZE (assuming no error)?

> +        if (ret >= 0) {
> +            for (int i = 0; i < numfds; ++i ) {

I don't think declaring the loop variable in the loop is allowed by Wine's coding standards.

> +                int32_t fd = infos[i].proc_fd;
> +                uint32_t type = infos[i].proc_fdtype;
> +                if (type == PROX_FDTYPE_SOCKET) {
> +                    TRACE("Closing socket with descriptor %d\n", fd);
> +                    close(fd);
> +                } 
> +            }
> +        }
> +        free(infos);
> +    }
> +#else
> +    FIXME("stub")
> +#endif
> +}
>  
>  /***********************************************************************
>   *      WSACleanup			(WS2_32.116)
> @@ -1458,8 +1490,14 @@ INT WINAPI WSACleanup(void)
>      if (num_startup) {
>          num_startup--;
>          TRACE("pending cleanups: %d\n", num_startup);
> +        if (num_startup == 0) {
> +            TRACE("cleaning up open sockets");
> +            close_open_sockets();
> +        }
>          return 0;
>      }
> +
> +    /* Close all sockets */

What's this last comment about?  Did you mean to put it above?

>      SetLastError(WSANOTINITIALISED);
>      return SOCKET_ERROR;
>  }
> 

-Ken