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

2019-03-05 Thread Marc-André Lureau
Hi

On Tue, Mar 5, 2019 at 3:07 PM Frediano Ziglio  wrote:
>
> > Hi
> >
> > On Wed, Feb 6, 2019 at 2:59 PM Frediano Ziglio  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 
> > > ---
> > >  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), );
> > >  }
> > > +
> > > +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*) , 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*) , ) < 0) {
> > > +goto cleanup;
> > > +}
> > > +if (connect(pairs[0], (struct sockaddr*) , sizeof(sa)) < 0) {
> > > +goto cleanup;
> > > +}
> > > +addrlen = sizeof(sa2);
> > > +pairs[1] = accept(s, (struct sockaddr*) , );
> > > +if (pairs[1] == INVALID_SOCKET) {
> > > +goto cleanup;
> > > +}
> > > +
> > > +/* check proper connection */
> > > +addrlen = sizeof(sa);
> > > +if (getsockname(pairs[0], (struct sockaddr*) , ) < 0) {
> > > +goto cleanup;
> > > +}
> > > +addrlen = sizeof(sa2);
> > > +if (getpeername(pairs[1], (struct sockaddr*) , ) < 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
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2019-03-05 Thread Frediano Ziglio
> Hi
> 
> On Wed, Feb 6, 2019 at 2:59 PM Frediano Ziglio  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 
> > ---
> >  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), );
> >  }
> > +
> > +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.

> > +{
> > +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*) , 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*) , ) < 0) {
> > +goto cleanup;
> > +}
> > +if (connect(pairs[0], (struct sockaddr*) , sizeof(sa)) < 0) {
> > +goto cleanup;
> > +}
> > +addrlen = sizeof(sa2);
> > +pairs[1] = accept(s, (struct sockaddr*) , );
> > +if (pairs[1] == INVALID_SOCKET) {
> > +goto cleanup;
> > +}
> > +
> > +/* check proper connection */
> > +addrlen = sizeof(sa);
> > +if (getsockname(pairs[0], (struct sockaddr*) , ) < 0) {
> > +goto cleanup;
> > +}
> > +addrlen = sizeof(sa2);
> > +if (getpeername(pairs[1], (struct sockaddr*) , ) < 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
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2019-03-04 Thread Marc-André Lureau
Hi

On Wed, Feb 6, 2019 at 2:59 PM Frediano Ziglio  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 
> ---
>  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), );
>  }
> +
> +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() ?

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)

> +{
> +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*) , 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*) , ) < 0) {
> +goto cleanup;
> +}
> +if (connect(pairs[0], (struct sockaddr*) , sizeof(sa)) < 0) {
> +goto cleanup;
> +}
> +addrlen = sizeof(sa2);
> +pairs[1] = accept(s, (struct sockaddr*) , );
> +if (pairs[1] == INVALID_SOCKET) {
> +goto cleanup;
> +}
> +
> +/* check proper connection */
> +addrlen = sizeof(sa);
> +if (getsockname(pairs[0], (struct sockaddr*) , ) < 0) {
> +goto cleanup;
> +}
> +addrlen = sizeof(sa2);
> +if (getpeername(pairs[1], (struct sockaddr*) , ) < 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_
> --
> 2.20.1
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



-- 
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2019-02-06 Thread Frediano Ziglio
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 
---
 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), );
 }
+
+int socket_newpair(int type, int protocol, int sv[2])
+{
+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*) , 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*) , ) < 0) {
+goto cleanup;
+}
+if (connect(pairs[0], (struct sockaddr*) , sizeof(sa)) < 0) {
+goto cleanup;
+}
+addrlen = sizeof(sa2);
+pairs[1] = accept(s, (struct sockaddr*) , );
+if (pairs[1] == INVALID_SOCKET) {
+goto cleanup;
+}
+
+/* check proper connection */
+addrlen = sizeof(sa);
+if (getsockname(pairs[0], (struct sockaddr*) , ) < 0) {
+goto cleanup;
+}
+addrlen = sizeof(sa2);
+if (getpeername(pairs[1], (struct sockaddr*) , ) < 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_
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel