Re: [Qemu-devel] [PATCH for-3.2 01/41] slirp: move socket pair creation in helper function

2018-11-19 Thread Samuel Thibault
Hello,

Marc-André Lureau, le mer. 14 nov. 2018 16:36:03 +0400, a ecrit:
> Originally, the patch was fixing a bunch of issues, but Peter beat me
> to it with earlier commit "slirp: fork_exec(): create and connect
> child socket before fork()".
> 
> Factor out socket pair creation, to simplify the fork_exec() code.
> Use the name socketpair_with_oob() since the code is actually similar
> to what socketpair() would do, except that it uses TCP sockets, for
> SLIRP to be able to call send with MSG_OOB (since SO_OOBINLINE is set,
> this could probably be faked instead on regular unix sockets though).
> 
> Signed-off-by: Marc-André Lureau 

Applied to my tree, thanks!

Samuel



[Qemu-devel] [PATCH for-3.2 01/41] slirp: move socket pair creation in helper function

2018-11-14 Thread Marc-André Lureau
Originally, the patch was fixing a bunch of issues, but Peter beat me
to it with earlier commit "slirp: fork_exec(): create and connect
child socket before fork()".

Factor out socket pair creation, to simplify the fork_exec() code.
Use the name socketpair_with_oob() since the code is actually similar
to what socketpair() would do, except that it uses TCP sockets, for
SLIRP to be able to call send with MSG_OOB (since SO_OOBINLINE is set,
this could probably be faked instead on regular unix sockets though).

Signed-off-by: Marc-André Lureau 
---
 slirp/misc.c | 142 +--
 1 file changed, 71 insertions(+), 71 deletions(-)

diff --git a/slirp/misc.c b/slirp/misc.c
index 4840187750..7362e14339 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -73,85 +73,92 @@ fork_exec(struct socket *so, const char *ex)
 
 #else
 
-/*
- * XXX This is ugly
- * We create and bind a socket, then fork off to another
- * process, which connects to this socket, after which we
- * exec the wanted program.  If something (strange) happens,
- * the accept() call could block us forever.
- */
+static int
+slirp_socketpair_with_oob(int sv[2])
+{
+struct sockaddr_in addr = {
+.sin_family = AF_INET,
+.sin_port = 0,
+.sin_addr.s_addr = INADDR_ANY,
+};
+socklen_t addrlen = sizeof(addr);
+int ret, s;
+
+sv[1] = -1;
+s = qemu_socket(AF_INET, SOCK_STREAM, 0);
+if (s < 0 || bind(s, (struct sockaddr *), addrlen) < 0 ||
+listen(s, 1) < 0 ||
+getsockname(s, (struct sockaddr *), ) < 0) {
+goto err;
+}
+
+sv[1] = qemu_socket(AF_INET, SOCK_STREAM, 0);
+if (sv[1] < 0) {
+goto err;
+}
+/*
+ * This connect won't block because we've already listen()ed on
+ * the server end (even though we won't accept() the connection
+ * until later on).
+ */
+do {
+ret = connect(sv[1], (struct sockaddr *), addrlen);
+} while (ret < 0 && errno == EINTR);
+if (ret < 0) {
+goto err;
+}
+
+do {
+sv[0] = accept(s, (struct sockaddr *), );
+} while (sv[0] < 0 && errno == EINTR);
+if (sv[0] < 0) {
+goto err;
+}
+
+closesocket(s);
+return 0;
+
+err:
+error_report("Error: slirp_socketpair(): %s", strerror(errno));
+if (s >= 0) {
+closesocket(s);
+}
+if (sv[1] >= 0) {
+closesocket(sv[1]);
+}
+return -1;
+}
+
 int
 fork_exec(struct socket *so, const char *ex)
 {
-int s, cs;
-struct sockaddr_in addr, csaddr;
-   socklen_t addrlen = sizeof(addr);
-socklen_t csaddrlen = sizeof(csaddr);
-   int opt;
char **argv;
-   int ret;
+   int opt, c, sp[2];
pid_t pid;
 
DEBUG_CALL("fork_exec");
DEBUG_ARG("so = %p", so);
DEBUG_ARG("ex = %p", ex);
 
-addr.sin_family = AF_INET;
-addr.sin_port = 0;
-addr.sin_addr.s_addr = INADDR_ANY;
-
-s = qemu_socket(AF_INET, SOCK_STREAM, 0);
-if (s < 0 || bind(s, (struct sockaddr *), addrlen) < 0 ||
-listen(s, 1) < 0) {
-error_report("Error: inet socket: %s", strerror(errno));
-if (s >= 0) {
-closesocket(s);
-}
+if (slirp_socketpair_with_oob(sp) < 0) {
 return 0;
 }
 
-if (getsockname(s, (struct sockaddr *), ) < 0) {
-closesocket(s);
-return 0;
-}
-cs = qemu_socket(AF_INET, SOCK_STREAM, 0);
-if (cs < 0) {
-closesocket(s);
-return 0;
-}
-csaddr.sin_addr = loopback_addr;
-/*
- * This connect won't block because we've already listen()ed on
- * the server end (even though we won't accept() the connection
- * until later on).
- */
-do {
-ret = connect(cs, (struct sockaddr *), csaddrlen);
-} while (ret < 0 && errno == EINTR);
-if (ret < 0) {
-closesocket(s);
-closesocket(cs);
-return 0;
-}
-
pid = fork();
switch(pid) {
 case -1:
error_report("Error: fork failed: %s", strerror(errno));
-closesocket(cs);
-   close(s);
+   closesocket(sp[0]);
+   closesocket(sp[1]);
return 0;
 
 case 0:
-setsid();
-
-   /* Set the DISPLAY */
-close(s);
-dup2(cs, 0);
-dup2(cs, 1);
-dup2(cs, 2);
-   for (s = getdtablesize() - 1; s >= 3; s--)
-  close(s);
+   setsid();
+   dup2(sp[1], 0);
+   dup2(sp[1], 1);
+   dup2(sp[1], 2);
+   for (c = getdtablesize() - 1; c >= 3; c--)
+  close(c);
 
 argv = g_strsplit(ex, " ", -1);
execvp(argv[0], (char **)argv);
@@ -163,19 +170,12 @@ fork_exec(struct socket *so, const char