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

[Dr. David Alan Gilbert]

* Peter Maydell (address@hidden) wrote:
> The code in sosendoob() assumes that slirp_send() always
> succeeds, but it might return an OS error code (for instance
> if the other end has disconnected). Catch these and return
> the caller either -1 on error or the number of urgent bytes
> actually written. (None of the callers check this return
> value currently, though.)
> 
> Signed-off-by: Peter Maydell <address@hidden>

That looks OK to me; the changes don't change the datastructure
usage at all - it's just the updated or so_urgc if it worked.


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

> ---
>  slirp/socket.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/slirp/socket.c b/slirp/socket.c
> index 3b49a69..a17caa9 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -345,33 +345,40 @@ sosendoob(struct socket *so)
>       if (sb->sb_rptr < sb->sb_wptr) {
>               /* We can send it directly */
>               n = slirp_send(so, sb->sb_rptr, so->so_urgc, (MSG_OOB)); /* 
> |MSG_DONTWAIT)); */
> -             so->so_urgc -= n;
> -
> -             DEBUG_MISC((dfd, " --- sent %d bytes urgent data, %d urgent 
> bytes left\n", n, so->so_urgc));
>       } else {
>               /*
>                * Since there's no sendv or sendtov like writev,
>                * we must copy all data to a linear buffer then
>                * send it all
>                */
> +             uint32_t urgc = so->so_urgc;
>               len = (sb->sb_data + sb->sb_datalen) - sb->sb_rptr;
> -             if (len > so->so_urgc) len = so->so_urgc;
> +             if (len > urgc) {
> +                     len = urgc;
> +             }
>               memcpy(buff, sb->sb_rptr, len);
> -             so->so_urgc -= len;
> -             if (so->so_urgc) {
> +             urgc -= len;
> +             if (urgc) {
>                       n = sb->sb_wptr - sb->sb_data;
> -                     if (n > so->so_urgc) n = so->so_urgc;
> +                     if (n > urgc) {
> +                             n = urgc;
> +                     }
>                       memcpy((buff + len), sb->sb_data, n);
> -                     so->so_urgc -= n;
>                       len += n;
>               }
>               n = slirp_send(so, buff, len, (MSG_OOB)); /* |MSG_DONTWAIT)); */
> +     }
> +
>  #ifdef DEBUG
> -             if (n != len)
> -                DEBUG_ERROR((dfd, "Didn't send all data urgently XXXXX\n"));
> +     if (n != len) {
> +             DEBUG_ERROR((dfd, "Didn't send all data urgently XXXXX\n"));
> +     }
>  #endif
> -             DEBUG_MISC((dfd, " ---2 sent %d bytes urgent data, %d urgent 
> bytes left\n", n, so->so_urgc));
> +     if (n < 0) {
> +             return n;
>       }
> +     so->so_urgc -= n;
> +     DEBUG_MISC((dfd, " ---2 sent %d bytes urgent data, %d urgent bytes 
> left\n", n, so->so_urgc));
>  
>       sb->sb_cc -= n;
>       sb->sb_rptr += n;
> -- 
> 2.7.4
> 
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK