Re: [qemu-s390x] [PATCH v3 12/25] xen: Let buffer_append() return the size consumed

[Marc-André Lureau]

Hi

On Wed, Feb 20, 2019 at 2:06 AM Philippe Mathieu-Daudé
<address@hidden> wrote:
>
> The buffer.size and buffer.consumed fields are only updated within the
> buffer_append() body. We can simply let buffer_append() return the
> difference (the buffer consumed).
>
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>

This is weird, why not introduce a buffer_size() function instead?

> ---
>  hw/char/xen_console.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index 083b2c8e2a..1a30014a11 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -48,7 +48,7 @@ struct XenConsole {
>      int               backlog;
>  };
>
> -static void buffer_append(struct XenConsole *con)
> +static ssize_t buffer_append(struct XenConsole *con)
>  {
>      struct buffer *buffer = &con->buffer;
>      XENCONS_RING_IDX cons, prod, size;
> @@ -59,8 +59,9 @@ static void buffer_append(struct XenConsole *con)
>      xen_mb();
>
>      size = prod - cons;
> -    if ((size == 0) || (size > sizeof(intf->out)))
> -        return;
> +    if ((size == 0) || (size > sizeof(intf->out))) {
> +        goto out;
> +    }
>
>      if ((buffer->capacity - buffer->size) < size) {
>          buffer->capacity += (size + 1024);
> @@ -89,6 +90,9 @@ static void buffer_append(struct XenConsole *con)
>          if (buffer->consumed > buffer->max_capacity - over)
>              buffer->consumed = buffer->max_capacity - over;
>      }
> +
> + out:
> +    return buffer->size - buffer->consumed;
>  }
>
>  static void buffer_advance(struct buffer *buffer, size_t len)
> @@ -281,8 +285,7 @@ static void con_event(struct XenLegacyDevice *xendev)
>      struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
>      ssize_t size;
>
> -    buffer_append(con);
> -    size = con->buffer.size - con->buffer.consumed;
> +    size = buffer_append(con);
>      if (size) {
>          xencons_send(con, size);
>      }
> --
> 2.20.1
>