Re: [Qemu-ppc] [RFC PATCH v3 13/25] xen: Let buffer_append() return a size_t

[Paul Durrant]

> -----Original Message-----
> From: Philippe Mathieu-Daudé [mailto:address@hidden
> Sent: 20 February 2019 01:02
> To: address@hidden; Prasad J Pandit <address@hidden>; Marc-
> André Lureau <address@hidden>; Paolo Bonzini
> <address@hidden>
> Cc: Jason Wang <address@hidden>; Anthony Perard
> <address@hidden>; address@hidden; Stefan Berger
> <address@hidden>; David Gibson <address@hidden>; Gerd
> Hoffmann <address@hidden>; Zhang Chen <address@hidden>; xen-
> address@hidden; Cornelia Huck <address@hidden>; Samuel
> Thibault <address@hidden>; Christian Borntraeger
> <address@hidden>; Amit Shah <address@hidden>; Li Zhijian
> <address@hidden>; Corey Minyard <address@hidden>; Michael S.
> Tsirkin <address@hidden>; Paul Durrant <address@hidden>; Halil
> Pasic <address@hidden>; Stefano Stabellini <address@hidden>;
> address@hidden; Pavel Dovgalyuk <address@hidden>;
> Philippe Mathieu-Daudé <address@hidden>
> Subject: [RFC PATCH v3 13/25] xen: Let buffer_append() return a size_t
> 
> To the Xen team: this is not trivial to me to demonstrate
> this assertion can never happen, but then the whole series
> is justified and I can convert qemu_chr_fe_write() to use
> size_t argument.
> Can you help me here?

I'm not particularly familiar with this bit of code but I can try...

> 
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  hw/char/xen_console.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index 1a30014a11..5b672a5a24 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -92,6 +92,7 @@ static ssize_t buffer_append(struct XenConsole *con)
>      }
> 
>   out:
> +    assert(buffer->size >= buffer->consumed);
>      return buffer->size - buffer->consumed;

I think this assertion is reasonable as:

- buffer_advance() appears to hit a termination condition when buffer->consumed 
== buffer->size. (Nothing checks for overflow which is bad, but that fact also 
lends weight to the assertion that consumed > size is a bug).
- if buffer->size ever exceeds buffer->max_capacity then both size and consumed 
are re-calculated such that consumed <= size.

  Paul

>  }
> 
> --
> 2.20.1