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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Philippe Mathieu-Daudé [mailto:philmd@xxxxxxxxxx]
> Sent: 20 February 2019 01:02
> To: qemu-devel@xxxxxxxxxx; Prasad J Pandit <pjp@xxxxxxxxxxxxxxxxx>; Marc-
> André Lureau <marcandre.lureau@xxxxxxxxxx>; Paolo Bonzini
> <pbonzini@xxxxxxxxxx>
> Cc: Jason Wang <jasowang@xxxxxxxxxx>; Anthony Perard
> <anthony.perard@xxxxxxxxxx>; qemu-ppc@xxxxxxxxxx; Stefan Berger
> <stefanb@xxxxxxxxxxxxx>; David Gibson <david@xxxxxxxxxxxxxxxxxxxxx>; Gerd
> Hoffmann <kraxel@xxxxxxxxxx>; Zhang Chen <zhangckid@xxxxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx; Cornelia Huck <cohuck@xxxxxxxxxx>; Samuel
> Thibault <samuel.thibault@xxxxxxxxxxxx>; Christian Borntraeger
> <borntraeger@xxxxxxxxxx>; Amit Shah <amit@xxxxxxxxxx>; Li Zhijian
> <lizhijian@xxxxxxxxxxxxxx>; Corey Minyard <minyard@xxxxxxx>; Michael S.
> Tsirkin <mst@xxxxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Halil
> Pasic <pasic@xxxxxxxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>;
> qemu-s390x@xxxxxxxxxx; Pavel Dovgalyuk <pavel.dovgaluk@xxxxxxxxx>;
> Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>
> 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é <philmd@xxxxxxxxxx>
> ---
>  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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel