[Spice-devel] [PATCH spice-server v4 08/20] sys-socket: Add socket_newpair utility

[Marc-André Lureau]

Hi

On Tue, Mar 5, 2019 at 3:07 PM Frediano Ziglio <fziglio at redhat.com> wrote:
>
> > Hi
> >
> > On Wed, Feb 6, 2019 at 2:59 PM Frediano Ziglio <fziglio at redhat.com> wrote:
> > >
> > > Allows to easier port socketpair.
> > > Windows does not have this function, we need to create a pair
> > > using 2 internet sockets and connecting one to the other.
> > >
> > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > ---
> > >  server/sys-socket.c | 75 +++++++++++++++++++++++++++++++++++++++++++++
> > >  server/sys-socket.h |  3 ++
> > >  2 files changed, 78 insertions(+)
> > >
> > > diff --git a/server/sys-socket.c b/server/sys-socket.c
> > > index 7ce5dab1..837da18e 100644
> > > --- a/server/sys-socket.c
> > > +++ b/server/sys-socket.c
> > > @@ -209,4 +209,79 @@ SPICE_CONSTRUCTOR_FUNC(socket_win32_init)
> > >      WSADATA wsaData;
> > >      WSAStartup(MAKEWORD(2, 2), &wsaData);
> > >  }
> > > +
> > > +int socket_newpair(int type, int protocol, int sv[2])
> >
> > overall, looks good.
> >
> > Since it's limited to inet protocols, why not remove "protocol"
> > argument, and call it socket_new_inet_pair() ?
> >
>
> it would make sense too, yes, I'll do.
>
> > If you are using this for worker thread communication, a windows pipe
> > would be a better fit. (and GIO provides stream abstraction, wing has
> > a namedpipe implementation - based on the one we used to have in
> > spice-gtk for controller communication)
> >
>
> We call some Qemu functions which needs sockets, so GIO and windows pipe
> won't work.
>

Can you give some context on expected the usage of this function then?
thanks

> > > +{
> > > +    struct sockaddr_in sa, sa2;
> > > +    socklen_t addrlen;
> > > +    SOCKET s, pairs[2];
> > > +
> > > +    if (!sv) {
> > > +        return -1;
> > > +    }
> > > +
> > > +    /* create a listener */
> > > +    s = socket(AF_INET, type, 0);
> > > +    if (s == INVALID_SOCKET) {
> > > +        return -1;
> > > +    }
> > > +
> > > +    pairs[1] = INVALID_SOCKET;
> > > +
> > > +    pairs[0] = socket(AF_INET, type, 0);
> > > +    if (pairs[0] == INVALID_SOCKET) {
> > > +        goto cleanup;
> > > +    }
> > > +
> > > +    /* bind to a random port */
> > > +    sa.sin_family = AF_INET;
> > > +    sa.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
> > > +    sa.sin_port = 0;
> > > +    if (bind(s, (struct sockaddr*) &sa, sizeof(sa)) < 0) {
> > > +        goto cleanup;
> > > +    }
> > > +    if (listen(s, 1) < 0) {
> > > +        goto cleanup;
> > > +    }
> > > +
> > > +    /* connect to kernel choosen port */
> > > +    addrlen = sizeof(sa);
> > > +    if (getsockname(s, (struct sockaddr*) &sa, &addrlen) < 0) {
> > > +        goto cleanup;
> > > +    }
> > > +    if (connect(pairs[0], (struct sockaddr*) &sa, sizeof(sa)) < 0) {
> > > +        goto cleanup;
> > > +    }
> > > +    addrlen = sizeof(sa2);
> > > +    pairs[1] = accept(s, (struct sockaddr*) &sa2, &addrlen);
> > > +    if (pairs[1] == INVALID_SOCKET) {
> > > +        goto cleanup;
> > > +    }
> > > +
> > > +    /* check proper connection */
> > > +    addrlen = sizeof(sa);
> > > +    if (getsockname(pairs[0], (struct sockaddr*) &sa, &addrlen) < 0) {
> > > +        goto cleanup;
> > > +    }
> > > +    addrlen = sizeof(sa2);
> > > +    if (getpeername(pairs[1], (struct sockaddr*) &sa2, &addrlen) < 0) {
> > > +        goto cleanup;
> > > +    }
> > > +    if (sa.sin_family != sa2.sin_family || sa.sin_port != sa2.sin_port
> > > +        || sa.sin_addr.s_addr != sa2.sin_addr.s_addr) {
> > > +        goto cleanup;
> > > +    }
> > > +
> > > +    closesocket(s);
> > > +    sv[0] = pairs[0];
> > > +    sv[1] = pairs[1];
> > > +    return 0;
> > > +
> > > +cleanup:
> > > +    socket_win32_set_errno();
> > > +    closesocket(s);
> > > +    closesocket(pairs[0]);
> > > +    closesocket(pairs[1]);
> > > +    return -1;
> > > +}
> > >  #endif
> > > diff --git a/server/sys-socket.h b/server/sys-socket.h
> > > index 65062571..3a3b7878 100644
> > > --- a/server/sys-socket.h
> > > +++ b/server/sys-socket.h
> > > @@ -134,6 +134,9 @@ socket_accept(int sock, struct sockaddr *addr, int
> > > *addrlen)
> > >  }
> > >  #undef accept
> > >  #define accept socket_accept
> > > +
> > > +int socket_newpair(int type, int protocol, int sv[2]);
> > > +#define socketpair(family, type, protocol, sv) socket_newpair(type,
> > > protocol, sv)
> > >  #endif
> > >
> > >  #endif // RED_SYS_SOCKET_H_
>
> Frediano



-- 
Marc-André Lureau