qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [RFC PATCH 04/56] char: Make ringbuf-read


From: Marc-André Lureau
Subject: Re: [Qemu-block] [Qemu-devel] [RFC PATCH 04/56] char: Make ringbuf-read size unsigned in QAPI/QMP
Date: Tue, 22 Aug 2017 13:32:42 +0200

Hi

On Mon, Aug 7, 2017 at 4:45 PM, Markus Armbruster <address@hidden> wrote:
> Sizes should use QAPI type 'size' (uint64_t).  ringbuf-read parameter
> @size is 'int' (int64_t).  qmp_ringbuf_read() rejects negative values,
> then implicitly converts to size_t.
>
> Change the parameter to 'size' and drop the check for negative values.
>
> ringbuf-read now accepts size values between 2^63 and 2^64-1.  It
> accepts negative values as before, because that's how the QObject
> input visitor works for backward compatibility.
>

Negative values over json will be implicitly converted to positive
values with this change, right? Or are they rejected earlier?

If so that is a change of behaviour that I am not sure is worth doing
now (without explicit protocol break), but I don't mind.

> The HMP command's size parameter remains uint32_t, as HMP args_type
> strings can't do uint64_t byte counts: 'l' is signed, and 'o'
> multiplies by 2^20.
>
> Signed-off-by: Markus Armbruster <address@hidden>

> ---
>  chardev/char-ringbuf.c | 11 +++--------
>  qapi-schema.json       |  2 +-
>  2 files changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/chardev/char-ringbuf.c b/chardev/char-ringbuf.c
> index df52b04..a9205ea 100644
> --- a/chardev/char-ringbuf.c
> +++ b/chardev/char-ringbuf.c
> @@ -65,10 +65,10 @@ static int ringbuf_chr_write(Chardev *chr, const uint8_t 
> *buf, int len)
>      return len;
>  }
>
> -static int ringbuf_chr_read(Chardev *chr, uint8_t *buf, int len)
> +static int ringbuf_chr_read(Chardev *chr, uint8_t *buf, size_t len)
>  {
>      RingBufChardev *d = RINGBUF_CHARDEV(chr);
> -    int i;
> +    size_t i;
>
>      qemu_mutex_lock(&chr->chr_write_lock);
>      for (i = 0; i < len && d->cons != d->prod; i++) {
> @@ -151,7 +151,7 @@ void qmp_ringbuf_write(const char *device, const char 
> *data,
>      }
>  }
>
> -char *qmp_ringbuf_read(const char *device, int64_t size,
> +char *qmp_ringbuf_read(const char *device, uint64_t size,
>                         bool has_format, enum DataFormat format,
>                         Error **errp)
>  {
> @@ -171,11 +171,6 @@ char *qmp_ringbuf_read(const char *device, int64_t size,
>          return NULL;
>      }
>
> -    if (size <= 0) {
> -        error_setg(errp, "size must be greater than zero");
> -        return NULL;
> -    }
> -
>      count = ringbuf_count(chr);
>      size = size > count ? count : size;
>      read_data = g_malloc(size + 1);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index febe70e..18ec301 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -543,7 +543,7 @@
>  #
>  ##
>  { 'command': 'ringbuf-read',
> -  'data': {'device': 'str', 'size': 'int', '*format': 'DataFormat'},
> +  'data': {'device': 'str', 'size': 'size', '*format': 'DataFormat'},
>    'returns': 'str' }
>
>  ##
> --
> 2.7.5
>
>



-- 
Marc-André Lureau



reply via email to

[Prev in Thread] Current Thread [Next in Thread]