Re: [Qemu-devel] [PATCH v1 17/21] osdep: add wrappers for socket functions

[Paolo Bonzini <pbonzini@redhat.com>]

On 09/03/2016 18:28, Daniel P. Berrange wrote:
> +static bool qemu_is_socket(int fd)
> +{
> +    int optval = 0;
> +    socklen_t optlen;
> +    optlen = sizeof(optval);
> +    return getsockopt(fd, SOL_SOCKET, SO_TYPE,
> +                      (char *)&optval, &optlen) == 0;

I think it's possible for a socket (which is actually a HANDLE) and a
file descriptor with the same numeric value to exist at the same time.
It's unlikely but it cannot be ruled out.

gnulib does this getsockopt (or at least could plausibly do it, I didn't
check :)) because it opens a libc file descriptor for every socket.
However, this only breaks ioctl and close and patch 18; neither ioctl
nor close's errors are ever checked in QEMU.
Doing the libc file descriptor in wrapping makes the select and poll
wrappers much more complex.  gnulib in fact even makes them work on
files, consoles, etc. besides pipes to provide a fuller POSIX layer.  In
my opinion this is beyond QEMU's scope and not really in line with
QEMU's attempts to use Win32 natively whenever applicable (e.g. in
qemu-char.c and block/raw-win32.c).

However, I'm okay with the other wrapping that you do in this patch.
Being able to drop socket_error() is a big improvement.

Paolo

> +}
> +
> +#undef ioctl
> +int qemu_ioctl_wrap(int fd, int req, void *val)
> +{
> +    int ret;
> +    if (qemu_is_socket(fd)) {
> +        ret = ioctlsocket(fd, req, val);
> +        if (ret < 0) {
> +            errno = socket_error();
> +        }
> +    } else {
> +        errno = EINVAL;
> +        ret = -1;
> +    }
> +    return ret;
> +}
> +
> +
> +#undef close
> +int qemu_close_wrap(int fd)
> +{
> +    int ret;
> +    if (qemu_is_socket(fd)) {
> +        ret = closesocket(fd);
> +        if (ret < 0) {
> +            errno = socket_error();
> +        }
> +    } else {
> +        ret = close(fd);
> +    }
> +    return ret;
> +}
> +
> +
> +#undef getsockopt
> +int qemu_getsockopt_wrap(int sockfd, int level, int optname,
> +                         void *optval, socklen_t *optlen)
> +{
> +    int ret;
> +    ret = getsockopt(sockfd, level, optname, optval, optlen);
> +    if (ret < 0) {
> +        errno = socket_error();
> +    }
> +    return ret;
> +}
> +
> +
> +#undef setsockopt
> +int qemu_setsockopt_wrap(int sockfd, int level, int optname,
> +                         const void *optval, socklen_t optlen)
> +{
> +    int ret;
> +    ret = setsockopt(sockfd, level, optname, optval, optlen);
> +    if (ret < 0) {
> +        errno = socket_error();
> +    }
> +    return ret;
> +}
> +
> +
> +#undef getpeername
> +int qemu_getpeername_wrap(int sockfd, struct sockaddr *addr,
> +                          socklen_t *addrlen)
> +{
> +    int ret;
> +    ret = getpeername(sockfd, addr, addrlen);
> +    if (ret < 0) {
> +        errno = socket_error();
> +    }
> +    return ret;
> +}
> +
> +
> +#undef getsockname
> +int qemu_getsockname_wrap(int sockfd, struct sockaddr *addr,
> +                          socklen_t *addrlen)
> +{
> +    int ret;
> +    ret = getsockname(sockfd, addr, addrlen);
> +    if (ret < 0) {
> +        errno = socket_error();
> +    }
> +    return ret;
> +}
> +
> +
> +#undef send
> +ssize_t qemu_send_wrap(int sockfd, const void *buf, size_t len, int flags)
> +{
> +    int ret;
> +    ret = send(sockfd, buf, len, flags);
> +    if (ret < 0) {
> +        errno = socket_error();
> +    }
> +    return ret;
> +}
> +
> +
> +#undef sendto
> +ssize_t qemu_sendto_wrap(int sockfd, const void *buf, size_t len, int flags,
> +                         const struct sockaddr *addr, socklen_t addrlen)
> +{
> +    int ret;
> +    ret = sendto(sockfd, buf, len, flags, addr, addrlen);
> +    if (ret < 0) {
> +        errno = socket_error();
> +    }
> +    return ret;
> +}
> +
> +
> +#undef recv
> +ssize_t qemu_recv_wrap(int sockfd, void *buf, size_t len, int flags)
> +{
> +    int ret;
> +    ret = recv(sockfd, buf, len, flags);
> +    if (ret < 0) {
> +        errno = socket_error();
> +    }
> +    return ret;
> +}
> +
> +
> +#undef recvfrom
> +ssize_t qemu_recvfrom_wrap(int sockfd, void *buf, size_t len, int flags,
> +                           struct sockaddr *addr, socklen_t *addrlen)
> +{
> +    int ret;
> +    ret = recvfrom(sockfd, buf, len, flags, addr, addrlen);
> +    if (ret < 0) {
> +        errno = socket_error();
> +    }
> +    return ret;
> +}
>