All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/4] slirp updates
@ 2018-11-10 14:09 Samuel Thibault
  2018-11-10 14:09 ` [Qemu-devel] [PULL 1/4] slirp: Don't pass possibly -1 fd to send() Samuel Thibault
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Samuel Thibault @ 2018-11-10 14:09 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Samuel Thibault, stefanha, jan.kiszka

The following changes since commit 160e5c22e55b3f775c2003dfc626fa872ee4a7a1:

  Merge remote-tracking branch 'remotes/gkurz/tags/for-upstream' into staging (2018-11-09 10:54:10 +0000)

are available in the Git repository at:

  https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to 5c75f3adbbfcdf8fae6e74875b44efb8d928974a:

  slirp: fork_exec(): create and connect child socket before fork() (2018-11-10 15:07:53 +0100)

----------------------------------------------------------------
slirp updates

Peter Maydell (4):
  slirp: Don't pass possibly -1 fd to send()
  slirp: Use g_new() to allocate sockets in socreate()
  slirp: Remove code that handles socreate() failure
  slirp: fork_exec(): create and connect child socket before fork()

----------------------------------------------------------------
Peter Maydell (4):
      slirp: Don't pass possibly -1 fd to send()
      slirp: Use g_new() to allocate sockets in socreate()
      slirp: Remove code that handles socreate() failure
      slirp: fork_exec(): create and connect child socket before fork()

 slirp/ip_icmp.c   |  2 +-
 slirp/misc.c      | 55 ++++++++++++++++++++++++++++++++++---------------------
 slirp/slirp.c     | 14 +++++++++++---
 slirp/socket.c    | 17 ++++++-----------
 slirp/tcp_input.c |  7 +++----
 slirp/tcp_subr.c  |  7 +------
 slirp/udp.c       |  6 ------
 slirp/udp6.c      |  3 ---
 8 files changed, 56 insertions(+), 55 deletions(-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PULL 1/4] slirp: Don't pass possibly -1 fd to send()
  2018-11-10 14:09 [Qemu-devel] [PULL 0/4] slirp updates Samuel Thibault
@ 2018-11-10 14:09 ` Samuel Thibault
  2018-11-10 14:09 ` [Qemu-devel] [PULL 2/4] slirp: Use g_new() to allocate sockets in socreate() Samuel Thibault
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Samuel Thibault @ 2018-11-10 14:09 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: stefanha, jan.kiszka, Samuel Thibault

From: Peter Maydell <peter.maydell@linaro.org>

Coverity complains (CID 1005726) that we might pass -1 as the fd
argument to send() in slirp_send(), because we previously checked for
"so->s == -1 && so->extra".  The case of "so->s == -1 but so->extra
NULL" should not in theory happen, but it is hard to guarantee
because various places in the code do so->s = qemu_socket(...) and so
will end up with so->s == -1 on failure, and not all the paths which
call that always throw away the socket in that case (eg
tcp_fconnect()).  So just check specifically for the condition and
fail slirp_send().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/slirp.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/slirp/slirp.c b/slirp/slirp.c
index 51de41fc02..3c3c03b22f 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -1091,6 +1091,17 @@ ssize_t slirp_send(struct socket *so, const void *buf, size_t len, int flags)
         return len;
     }
 
+    if (so->s == -1) {
+        /*
+         * This should in theory not happen but it is hard to be
+         * sure because some code paths will end up with so->s == -1
+         * on a failure but don't dispose of the struct socket.
+         * Check specifically, so we don't pass -1 to send().
+         */
+        errno = EBADF;
+        return -1;
+    }
+
     return send(so->s, buf, len, flags);
 }
 
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PULL 2/4] slirp: Use g_new() to allocate sockets in socreate()
  2018-11-10 14:09 [Qemu-devel] [PULL 0/4] slirp updates Samuel Thibault
  2018-11-10 14:09 ` [Qemu-devel] [PULL 1/4] slirp: Don't pass possibly -1 fd to send() Samuel Thibault
@ 2018-11-10 14:09 ` Samuel Thibault
  2018-11-10 14:09 ` [Qemu-devel] [PULL 3/4] slirp: Remove code that handles socreate() failure Samuel Thibault
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Samuel Thibault @ 2018-11-10 14:09 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: stefanha, jan.kiszka, Samuel Thibault

From: Peter Maydell <peter.maydell@linaro.org>

The slirp socreate() function can only fail if the attempt
to malloc() the struct socket fails. Switch to using
g_new() instead, which will allow us to remove the
error-handling code from its callers.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/socket.c    | 14 ++++++--------
 slirp/tcp_input.c |  4 ++--
 slirp/tcp_subr.c  |  2 +-
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/slirp/socket.c b/slirp/socket.c
index 322383a1f9..35a9a14565 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -46,17 +46,15 @@ struct socket *solookup(struct socket **last, struct socket *head,
 struct socket *
 socreate(Slirp *slirp)
 {
-  struct socket *so;
+    struct socket *so = g_new(struct socket, 1);
 
-  so = (struct socket *)malloc(sizeof(struct socket));
-  if(so) {
     memset(so, 0, sizeof(struct socket));
     so->so_state = SS_NOFDREF;
     so->s = -1;
     so->slirp = slirp;
     so->pollfds_idx = -1;
-  }
-  return(so);
+
+    return so;
 }
 
 /*
@@ -110,7 +108,7 @@ sofree(struct socket *so)
   if (so->so_tcpcb) {
       free(so->so_tcpcb);
   }
-  free(so);
+  g_free(so);
 }
 
 size_t sopreprbuf(struct socket *so, struct iovec *iov, int *np)
@@ -721,8 +719,8 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,
 
 	/* Don't tcp_attach... we don't need so_snd nor so_rcv */
 	if ((so->so_tcpcb = tcp_newtcpcb(so)) == NULL) {
-		free(so);
-		return NULL;
+            g_free(so);
+            return NULL;
 	}
 	insque(so, &slirp->tcb);
 
diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c
index 07bcbdb2dd..4f79c95fdb 100644
--- a/slirp/tcp_input.c
+++ b/slirp/tcp_input.c
@@ -432,8 +432,8 @@ findso:
 	  if ((so = socreate(slirp)) == NULL)
 	    goto dropwithreset;
 	  if (tcp_attach(so) < 0) {
-	    free(so); /* Not sofree (if it failed, it's not insqued) */
-	    goto dropwithreset;
+            g_free(so); /* Not sofree (if it failed, it's not insqued) */
+            goto dropwithreset;
 	  }
 
 	  sbreserve(&so->so_snd, TCP_SNDSPACE);
diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index 8d0f94b75f..0270c89ae3 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -475,7 +475,7 @@ void tcp_connect(struct socket *inso)
             return;
         }
         if (tcp_attach(so) < 0) {
-            free(so); /* NOT sofree */
+            g_free(so); /* NOT sofree */
             return;
         }
         so->lhost = inso->lhost;
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PULL 3/4] slirp: Remove code that handles socreate() failure
  2018-11-10 14:09 [Qemu-devel] [PULL 0/4] slirp updates Samuel Thibault
  2018-11-10 14:09 ` [Qemu-devel] [PULL 1/4] slirp: Don't pass possibly -1 fd to send() Samuel Thibault
  2018-11-10 14:09 ` [Qemu-devel] [PULL 2/4] slirp: Use g_new() to allocate sockets in socreate() Samuel Thibault
@ 2018-11-10 14:09 ` Samuel Thibault
  2018-11-10 14:09 ` [Qemu-devel] [PULL 4/4] slirp: fork_exec(): create and connect child socket before fork() Samuel Thibault
  2018-11-12 10:58 ` [Qemu-devel] [PULL 0/4] slirp updates Peter Maydell
  4 siblings, 0 replies; 11+ messages in thread
From: Samuel Thibault @ 2018-11-10 14:09 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: stefanha, jan.kiszka, Samuel Thibault

From: Peter Maydell <peter.maydell@linaro.org>

Now that socreate() can never fail, we can remove the code
that was trying to handle that situation.

In particular this removes code in tcp_connect() that
provoked Coverity to complain (CID 1005724): in
 closesocket(accept(inso->s, (struct sockaddr *)&addr, &addrlen));
if the accept() call fails then we pass closesocket() -1
instead of a valid file descriptor.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/ip_icmp.c   | 2 +-
 slirp/slirp.c     | 3 ---
 slirp/socket.c    | 3 ---
 slirp/tcp_input.c | 3 +--
 slirp/tcp_subr.c  | 5 -----
 slirp/udp.c       | 6 ------
 slirp/udp6.c      | 3 ---
 7 files changed, 2 insertions(+), 23 deletions(-)

diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c
index da100d1f55..9210eef3f3 100644
--- a/slirp/ip_icmp.c
+++ b/slirp/ip_icmp.c
@@ -160,7 +160,7 @@ icmp_input(struct mbuf *m, int hlen)
     } else {
       struct socket *so;
       struct sockaddr_storage addr;
-      if ((so = socreate(slirp)) == NULL) goto freeit;
+      so = socreate(slirp);
       if (icmp_send(so, m, hlen) == 0) {
         return;
       }
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 3c3c03b22f..322edf51eb 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -1477,9 +1477,6 @@ static int slirp_state_load(QEMUFile *f, void *opaque, int version_id)
         int ret;
         struct socket *so = socreate(slirp);
 
-        if (!so)
-            return -ENOMEM;
-
         ret = vmstate_load_state(f, &vmstate_slirp_socket, so, version_id);
 
         if (ret < 0)
diff --git a/slirp/socket.c b/slirp/socket.c
index 35a9a14565..c01d8696af 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -713,9 +713,6 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,
 	DEBUG_ARG("flags = %x", flags);
 
 	so = socreate(slirp);
-	if (!so) {
-	  return NULL;
-	}
 
 	/* Don't tcp_attach... we don't need so_snd nor so_rcv */
 	if ((so->so_tcpcb = tcp_newtcpcb(so)) == NULL) {
diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c
index 4f79c95fdb..d073ef9525 100644
--- a/slirp/tcp_input.c
+++ b/slirp/tcp_input.c
@@ -429,8 +429,7 @@ findso:
 	  if ((tiflags & (TH_SYN|TH_FIN|TH_RST|TH_URG|TH_ACK)) != TH_SYN)
 	    goto dropwithreset;
 
-	  if ((so = socreate(slirp)) == NULL)
-	    goto dropwithreset;
+          so = socreate(slirp);
 	  if (tcp_attach(so) < 0) {
             g_free(so); /* Not sofree (if it failed, it's not insqued) */
             goto dropwithreset;
diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index 0270c89ae3..fa61349cbb 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -469,11 +469,6 @@ void tcp_connect(struct socket *inso)
         so = inso;
     } else {
         so = socreate(slirp);
-        if (so == NULL) {
-            /* If it failed, get rid of the pending connection */
-            closesocket(accept(inso->s, (struct sockaddr *)&addr, &addrlen));
-            return;
-        }
         if (tcp_attach(so) < 0) {
             g_free(so); /* NOT sofree */
             return;
diff --git a/slirp/udp.c b/slirp/udp.c
index e5bf065bf2..c47870a61b 100644
--- a/slirp/udp.c
+++ b/slirp/udp.c
@@ -171,9 +171,6 @@ udp_input(register struct mbuf *m, int iphlen)
 	   * create one
 	   */
 	  so = socreate(slirp);
-	  if (!so) {
-	      goto bad;
-	  }
 	  if (udp_attach(so, AF_INET) == -1) {
 	    DEBUG_MISC((dfd," udp_attach errno = %d-%s\n",
 			errno,strerror(errno)));
@@ -331,9 +328,6 @@ udp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,
 	socklen_t addrlen = sizeof(struct sockaddr_in);
 
 	so = socreate(slirp);
-	if (!so) {
-	    return NULL;
-	}
 	so->s = qemu_socket(AF_INET,SOCK_DGRAM,0);
         if (so->s < 0) {
             sofree(so);
diff --git a/slirp/udp6.c b/slirp/udp6.c
index 7c4a6b003a..986010f0d3 100644
--- a/slirp/udp6.c
+++ b/slirp/udp6.c
@@ -91,9 +91,6 @@ void udp6_input(struct mbuf *m)
     if (so == NULL) {
         /* If there's no socket for this packet, create one. */
         so = socreate(slirp);
-        if (!so) {
-            goto bad;
-        }
         if (udp_attach(so, AF_INET6) == -1) {
             DEBUG_MISC((dfd, " udp6_attach errno = %d-%s\n",
                         errno, strerror(errno)));
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PULL 4/4] slirp: fork_exec(): create and connect child socket before fork()
  2018-11-10 14:09 [Qemu-devel] [PULL 0/4] slirp updates Samuel Thibault
                   ` (2 preceding siblings ...)
  2018-11-10 14:09 ` [Qemu-devel] [PULL 3/4] slirp: Remove code that handles socreate() failure Samuel Thibault
@ 2018-11-10 14:09 ` Samuel Thibault
  2018-11-12 10:58 ` [Qemu-devel] [PULL 0/4] slirp updates Peter Maydell
  4 siblings, 0 replies; 11+ messages in thread
From: Samuel Thibault @ 2018-11-10 14:09 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: stefanha, jan.kiszka, Samuel Thibault

From: Peter Maydell <peter.maydell@linaro.org>

Currently fork_exec() fork()s, and then creates and connects the
child socket which it uses for communication with the parent in
the child process. This is awkward because the child has no
mechanism to report failure back to the parent, which might end
up blocked forever in accept(). The child code also has an issue
pointed out by Coverity (CID 1005727), where if the qemu_socket()
call fails it will pass -1 as a file descriptor to connect().

Fix these issues by moving the creation of the child's end of
the socket to before the fork(), where we are in a position to
handle a possible failure.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/misc.c | 55 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/slirp/misc.c b/slirp/misc.c
index 260187b6b6..57bdd808e2 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -85,9 +85,10 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
 int
 fork_exec(struct socket *so, const char *ex, int do_pty)
 {
-	int s;
-	struct sockaddr_in addr;
+        int s, cs;
+        struct sockaddr_in addr, csaddr;
 	socklen_t addrlen = sizeof(addr);
+        socklen_t csaddrlen = sizeof(csaddr);
 	int opt;
 	const char *argv[256];
 	/* don't want to clobber the original */
@@ -120,10 +121,35 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
 		}
 	}
 
+        if (getsockname(s, (struct sockaddr *)&csaddr, &csaddrlen) < 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 *)&csaddr, 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);
 		return 0;
 
@@ -131,21 +157,10 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
                 setsid();
 
 		/* Set the DISPLAY */
-                getsockname(s, (struct sockaddr *)&addr, &addrlen);
                 close(s);
-                /*
-                 * Connect to the socket
-                 * XXX If any of these fail, we're in trouble!
-                 */
-                s = qemu_socket(AF_INET, SOCK_STREAM, 0);
-                addr.sin_addr = loopback_addr;
-                do {
-                    ret = connect(s, (struct sockaddr *)&addr, addrlen);
-                } while (ret < 0 && errno == EINTR);
-
-		dup2(s, 0);
-		dup2(s, 1);
-		dup2(s, 2);
+                dup2(cs, 0);
+                dup2(cs, 1);
+                dup2(cs, 2);
 		for (s = getdtablesize() - 1; s >= 3; s--)
 		   close(s);
 
@@ -178,12 +193,10 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
 
 	 default:
 		qemu_add_child_watch(pid);
+                closesocket(cs);
                 /*
-                 * XXX this could block us...
-                 * XXX Should set a timer here, and if accept() doesn't
-                 * return after X seconds, declare it a failure
-                 * The only reason this will block forever is if socket()
-                 * of connect() fail in the child process
+                 * This should never block, because we already connect()ed
+                 * on the child end before we forked.
                  */
                 do {
                     so->s = accept(s, (struct sockaddr *)&addr, &addrlen);
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PULL 0/4] slirp updates
  2018-11-10 14:09 [Qemu-devel] [PULL 0/4] slirp updates Samuel Thibault
                   ` (3 preceding siblings ...)
  2018-11-10 14:09 ` [Qemu-devel] [PULL 4/4] slirp: fork_exec(): create and connect child socket before fork() Samuel Thibault
@ 2018-11-12 10:58 ` Peter Maydell
  4 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2018-11-12 10:58 UTC (permalink / raw)
  To: Samuel Thibault; +Cc: QEMU Developers, Stefan Hajnoczi, Jan Kiszka

On 10 November 2018 at 14:09, Samuel Thibault
<samuel.thibault@ens-lyon.org> wrote:
> The following changes since commit 160e5c22e55b3f775c2003dfc626fa872ee4a7a1:
>
>   Merge remote-tracking branch 'remotes/gkurz/tags/for-upstream' into staging (2018-11-09 10:54:10 +0000)
>
> are available in the Git repository at:
>
>   https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
>
> for you to fetch changes up to 5c75f3adbbfcdf8fae6e74875b44efb8d928974a:
>
>   slirp: fork_exec(): create and connect child socket before fork() (2018-11-10 15:07:53 +0100)
>
> ----------------------------------------------------------------
> slirp updates
>
> Peter Maydell (4):
>   slirp: Don't pass possibly -1 fd to send()
>   slirp: Use g_new() to allocate sockets in socreate()
>   slirp: Remove code that handles socreate() failure
>   slirp: fork_exec(): create and connect child socket before fork()
>
> ----------------------------------------------------------------
> Peter Maydell (4):
>       slirp: Don't pass possibly -1 fd to send()
>       slirp: Use g_new() to allocate sockets in socreate()
>       slirp: Remove code that handles socreate() failure
>       slirp: fork_exec(): create and connect child socket before fork()

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PULL 0/4] slirp updates
  2017-07-15 12:30 Samuel Thibault
  2017-07-15 12:40 ` no-reply
@ 2017-07-17 10:14 ` Peter Maydell
  1 sibling, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2017-07-17 10:14 UTC (permalink / raw)
  To: Samuel Thibault; +Cc: QEMU Developers, Stefan Hajnoczi, Jan Kiszka

On 15 July 2017 at 13:30, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> warning: redirection vers https://people.debian.org/~sthibault/qemu.git/
> The following changes since commit 4871b51b9241b10f4fd8e04bbb21577886795e25:
>
>   vmgenid-test: use boot-sector infrastructure (2017-07-14 17:03:03 +0100)
>
> are available in the git repository at:
>
>   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
>
> for you to fetch changes up to 75cb298d905030fca897ea1d80e409c7f7e3e5ea:
>
>   slirp: Handle error returns from sosendoob() (2017-07-15 14:28:25 +0200)
>
> ----------------------------------------------------------------
> slirp updates
>
> ----------------------------------------------------------------
> Marc-André Lureau (1):
>       slirp: use DIV_ROUND_UP
>
> Peter Maydell (3):
>       slirp: fork_exec(): Don't close() a negative number in fork_exec()
>       slirp: Handle error returns from slirp_send() in sosendoob()
>       slirp: Handle error returns from sosendoob()
>
>  slirp/ip6.h    |  6 +++---
>  slirp/misc.c   |  4 +++-
>  slirp/sbuf.c   |  2 +-
>  slirp/socket.c | 52 +++++++++++++++++++++++++++++++++++-----------------
>  4 files changed, 42 insertions(+), 22 deletions(-)

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PULL 0/4] slirp updates
  2017-07-15 12:30 Samuel Thibault
@ 2017-07-15 12:40 ` no-reply
  2017-07-17 10:14 ` Peter Maydell
  1 sibling, 0 replies; 11+ messages in thread
From: no-reply @ 2017-07-15 12:40 UTC (permalink / raw)
  To: samuel.thibault; +Cc: famz, qemu-devel, stefanha, jan.kiszka

Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PULL 0/4] slirp updates
Message-id: 20170715123057.8529-1-samuel.thibault@ens-lyon.org
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20170715123057.8529-1-samuel.thibault@ens-lyon.org -> patchew/20170715123057.8529-1-samuel.thibault@ens-lyon.org
Switched to a new branch 'test'
d550cce slirp: Handle error returns from sosendoob()
3ccee6a slirp: Handle error returns from slirp_send() in sosendoob()
17f0f61 slirp: fork_exec(): Don't close() a negative number in fork_exec()
861877a6 slirp: use DIV_ROUND_UP

=== OUTPUT BEGIN ===
Checking PATCH 1/4: slirp: use DIV_ROUND_UP...
Checking PATCH 2/4: slirp: fork_exec(): Don't close() a negative number in fork_exec()...
ERROR: code indent should never use tabs
#27: FILE: slirp/misc.c:115:
+^I^I^Iif (s >= 0) {$

ERROR: code indent should never use tabs
#28: FILE: slirp/misc.c:116:
+^I^I^I    closesocket(s);$

ERROR: code indent should never use tabs
#29: FILE: slirp/misc.c:117:
+^I^I^I}$

total: 3 errors, 0 warnings, 10 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/4: slirp: Handle error returns from slirp_send() in sosendoob()...
ERROR: code indent should never use tabs
#35: FILE: slirp/socket.c:354:
+^I^Iuint32_t urgc = so->so_urgc;$

ERROR: code indent should never use tabs
#38: FILE: slirp/socket.c:356:
+^I^Iif (len > urgc) {$

ERROR: code indent should never use tabs
#39: FILE: slirp/socket.c:357:
+^I^I^Ilen = urgc;$

ERROR: code indent should never use tabs
#40: FILE: slirp/socket.c:358:
+^I^I}$

ERROR: code indent should never use tabs
#44: FILE: slirp/socket.c:360:
+^I^Iurgc -= len;$

ERROR: code indent should never use tabs
#45: FILE: slirp/socket.c:361:
+^I^Iif (urgc) {$

ERROR: code indent should never use tabs
#48: FILE: slirp/socket.c:363:
+^I^I^Iif (n > urgc) {$

ERROR: code indent should never use tabs
#49: FILE: slirp/socket.c:364:
+^I^I^I^In = urgc;$

ERROR: code indent should never use tabs
#50: FILE: slirp/socket.c:365:
+^I^I^I}$

ERROR: code indent should never use tabs
#56: FILE: slirp/socket.c:370:
+^I}$

ERROR: code indent should never use tabs
#61: FILE: slirp/socket.c:373:
+^Iif (n != len) {$

ERROR: code indent should never use tabs
#62: FILE: slirp/socket.c:374:
+^I^IDEBUG_ERROR((dfd, "Didn't send all data urgently XXXXX\n"));$

ERROR: code indent should never use tabs
#63: FILE: slirp/socket.c:375:
+^I}$

ERROR: code indent should never use tabs
#66: FILE: slirp/socket.c:377:
+^Iif (n < 0) {$

ERROR: code indent should never use tabs
#67: FILE: slirp/socket.c:378:
+^I^Ireturn n;$

ERROR: code indent should never use tabs
#69: FILE: slirp/socket.c:380:
+^Iso->so_urgc -= n;$

ERROR: line over 90 characters
#70: FILE: slirp/socket.c:381:
+	DEBUG_MISC((dfd, " ---2 sent %d bytes urgent data, %d urgent bytes left\n", n, so->so_urgc));

ERROR: code indent should never use tabs
#70: FILE: slirp/socket.c:381:
+^IDEBUG_MISC((dfd, " ---2 sent %d bytes urgent data, %d urgent bytes left\n", n, so->so_urgc));$

total: 18 errors, 0 warnings, 51 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 4/4: slirp: Handle error returns from sosendoob()...
ERROR: code indent should never use tabs
#27: FILE: slirp/sbuf.c:94:
+^I^I(void)sosendoob(so);$

ERROR: code indent should never use tabs
#40: FILE: slirp/socket.c:407:
+^I^Iuint32_t expected = so->so_urgc;$

ERROR: code indent should never use tabs
#41: FILE: slirp/socket.c:408:
+^I^Iif (sosendoob(so) < expected) {$

ERROR: code indent should never use tabs
#42: FILE: slirp/socket.c:409:
+^I^I^I/* Treat a short write as a fatal error too,$

ERROR: code indent should never use tabs
#43: FILE: slirp/socket.c:410:
+^I^I^I * rather than continuing on and sending the urgent$

ERROR: code indent should never use tabs
#44: FILE: slirp/socket.c:411:
+^I^I^I * data as if it were non-urgent and leaving the$

ERROR: code indent should never use tabs
#45: FILE: slirp/socket.c:412:
+^I^I^I * so_urgc count wrong.$

ERROR: code indent should never use tabs
#46: FILE: slirp/socket.c:413:
+^I^I^I */$

ERROR: code indent should never use tabs
#47: FILE: slirp/socket.c:414:
+^I^I^Igoto err_disconnected;$

ERROR: code indent should never use tabs
#48: FILE: slirp/socket.c:415:
+^I^I}$

ERROR: code indent should never use tabs
#61: FILE: slirp/socket.c:459:
+^I^Igoto err_disconnected;$

WARNING: line over 80 characters
#71: FILE: slirp/socket.c:488:
+	DEBUG_MISC((dfd, " --- sowrite disconnected, so->so_state = %x, errno = %d\n",

ERROR: code indent should never use tabs
#71: FILE: slirp/socket.c:488:
+^IDEBUG_MISC((dfd, " --- sowrite disconnected, so->so_state = %x, errno = %d\n",$

ERROR: code indent should never use tabs
#72: FILE: slirp/socket.c:489:
+^I^I    so->so_state, errno));$

ERROR: code indent should never use tabs
#73: FILE: slirp/socket.c:490:
+^Isofcantsendmore(so);$

ERROR: code indent should never use tabs
#74: FILE: slirp/socket.c:491:
+^Itcp_sockclosed(sototcpcb(so));$

ERROR: code indent should never use tabs
#75: FILE: slirp/socket.c:492:
+^Ireturn -1;$

total: 16 errors, 1 warnings, 49 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PULL 0/4] slirp updates
@ 2017-07-15 12:30 Samuel Thibault
  2017-07-15 12:40 ` no-reply
  2017-07-17 10:14 ` Peter Maydell
  0 siblings, 2 replies; 11+ messages in thread
From: Samuel Thibault @ 2017-07-15 12:30 UTC (permalink / raw)
  To: qemu-devel; +Cc: Samuel Thibault, stefanha, jan.kiszka

warning: redirection vers https://people.debian.org/~sthibault/qemu.git/
The following changes since commit 4871b51b9241b10f4fd8e04bbb21577886795e25:

  vmgenid-test: use boot-sector infrastructure (2017-07-14 17:03:03 +0100)

are available in the git repository at:

  http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to 75cb298d905030fca897ea1d80e409c7f7e3e5ea:

  slirp: Handle error returns from sosendoob() (2017-07-15 14:28:25 +0200)

----------------------------------------------------------------
slirp updates

----------------------------------------------------------------
Marc-André Lureau (1):
      slirp: use DIV_ROUND_UP

Peter Maydell (3):
      slirp: fork_exec(): Don't close() a negative number in fork_exec()
      slirp: Handle error returns from slirp_send() in sosendoob()
      slirp: Handle error returns from sosendoob()

 slirp/ip6.h    |  6 +++---
 slirp/misc.c   |  4 +++-
 slirp/sbuf.c   |  2 +-
 slirp/socket.c | 52 +++++++++++++++++++++++++++++++++++-----------------
 4 files changed, 42 insertions(+), 22 deletions(-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PULL 0/4] slirp updates
  2016-05-16 19:25 Samuel Thibault
@ 2016-05-17  9:35 ` Peter Maydell
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2016-05-17  9:35 UTC (permalink / raw)
  To: Samuel Thibault; +Cc: QEMU Developers, J. Kiszka

On 16 May 2016 at 20:25, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
>   Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20160513-1' into staging (2016-05-13 13:39:38 +0100)
>
> are available in the git repository at:
>
>   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
>
> for you to fetch changes up to 9892663dc486755b5534ff8a77913edc5ea28c79:
>
>   slirp: Clean up osdep.h related header inclusions (2016-05-16 21:01:16 +0200)
>
> ----------------------------------------------------------------
> slirp updates
>
> ----------------------------------------------------------------
> Thomas Huth (4):
>       slirp: Clean up slirp_config.h
>       slirp: Remove obsolete backward-compatibility cruft
>       slirp: Remove some unused code from slirp.h
>       slirp: Clean up osdep.h related header inclusions

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PULL 0/4] slirp updates
@ 2016-05-16 19:25 Samuel Thibault
  2016-05-17  9:35 ` Peter Maydell
  0 siblings, 1 reply; 11+ messages in thread
From: Samuel Thibault @ 2016-05-16 19:25 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Samuel Thibault, jan.kiszka

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20160513-1' into staging (2016-05-13 13:39:38 +0100)

are available in the git repository at:

  http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to 9892663dc486755b5534ff8a77913edc5ea28c79:

  slirp: Clean up osdep.h related header inclusions (2016-05-16 21:01:16 +0200)

----------------------------------------------------------------
slirp updates

----------------------------------------------------------------
Thomas Huth (4):
      slirp: Clean up slirp_config.h
      slirp: Remove obsolete backward-compatibility cruft
      slirp: Remove some unused code from slirp.h
      slirp: Clean up osdep.h related header inclusions

 slirp/ip6_icmp.c     |  1 -
 slirp/ip_input.c     |  1 -
 slirp/misc.c         | 21 -----------
 slirp/slirp.h        | 50 --------------------------
 slirp/slirp_config.h | 99 ----------------------------------------------------
 slirp/udp6.c         |  1 -
 6 files changed, 173 deletions(-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-11-12 10:59 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-10 14:09 [Qemu-devel] [PULL 0/4] slirp updates Samuel Thibault
2018-11-10 14:09 ` [Qemu-devel] [PULL 1/4] slirp: Don't pass possibly -1 fd to send() Samuel Thibault
2018-11-10 14:09 ` [Qemu-devel] [PULL 2/4] slirp: Use g_new() to allocate sockets in socreate() Samuel Thibault
2018-11-10 14:09 ` [Qemu-devel] [PULL 3/4] slirp: Remove code that handles socreate() failure Samuel Thibault
2018-11-10 14:09 ` [Qemu-devel] [PULL 4/4] slirp: fork_exec(): create and connect child socket before fork() Samuel Thibault
2018-11-12 10:58 ` [Qemu-devel] [PULL 0/4] slirp updates Peter Maydell
  -- strict thread matches above, loose matches on Subject: below --
2017-07-15 12:30 Samuel Thibault
2017-07-15 12:40 ` no-reply
2017-07-17 10:14 ` Peter Maydell
2016-05-16 19:25 Samuel Thibault
2016-05-17  9:35 ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.