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

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Peter Maydell (peter.maydell@linaro.org) 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 <peter.maydell@linaro.org>

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 <dgilbert@redhat.com>

> ---
>  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 / dgilbert@redhat.com / Manchester, UK