Re: [Qemu-devel] [PATCH v2 2/2] slirp: Handle error returns from sosendoob()

[Dr. David Alan Gilbert]

* Peter Maydell (address@hidden) wrote:
> sosendoob() can return a failure code, but all its callers ignore it.
> This is OK in sbappend(), as the comment there states -- we will try
> again later in sowrite(). Add a (void) cast to tell Coverity so.
> In sowrite() we do need to check the return value -- we should handle
> a write failure in sosendoob() the same way we handle a write failure
> for the normal data.
> 
> Signed-off-by: Peter Maydell <address@hidden>
> ---
>  slirp/sbuf.c   |  2 +-
>  slirp/socket.c | 23 +++++++++++++++++------
>  2 files changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/slirp/sbuf.c b/slirp/sbuf.c
> index 10119d3..912f235 100644
> --- a/slirp/sbuf.c
> +++ b/slirp/sbuf.c
> @@ -91,7 +91,7 @@ sbappend(struct socket *so, struct mbuf *m)
>       if (so->so_urgc) {
>               sbappendsb(&so->so_rcv, m);
>               m_free(m);
> -             sosendoob(so);
> +             (void)sosendoob(so);
>               return;
>       }
>  
> diff --git a/slirp/socket.c b/slirp/socket.c
> index a17caa9..ecec029 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -404,7 +404,15 @@ sowrite(struct socket *so)
>       DEBUG_ARG("so = %p", so);
>  
>       if (so->so_urgc) {
> -             sosendoob(so);
> +             uint32_t expected = so->so_urgc;
> +             if (sosendoob(so) < expected) {

Oops, yes that is better.

Reviewed-by: Dr. David Alan Gilbert <address@hidden>

> +                     /* Treat a short write as a fatal error too,
> +                      * rather than continuing on and sending the urgent
> +                      * data as if it were non-urgent and leaving the
> +                      * so_urgc count wrong.
> +                      */
> +                     goto err_disconnected;
> +             }
>               if (sb->sb_cc == 0)
>                       return 0;
>       }
> @@ -448,11 +456,7 @@ sowrite(struct socket *so)
>               return 0;
>  
>       if (nn <= 0) {
> -             DEBUG_MISC((dfd, " --- sowrite disconnected, so->so_state = %x, 
> errno = %d\n",
> -                     so->so_state, errno));
> -             sofcantsendmore(so);
> -             tcp_sockclosed(sototcpcb(so));
> -             return -1;
> +             goto err_disconnected;
>       }
>  
>  #ifndef HAVE_READV
> @@ -479,6 +483,13 @@ sowrite(struct socket *so)
>               sofcantsendmore(so);
>  
>       return nn;
> +
> +err_disconnected:
> +     DEBUG_MISC((dfd, " --- sowrite disconnected, so->so_state = %x, errno = 
> %d\n",
> +                 so->so_state, errno));
> +     sofcantsendmore(so);
> +     tcp_sockclosed(sototcpcb(so));
> +     return -1;
>  }
>  
>  /*
> -- 
> 2.7.4
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK