Re: [qemu-s390x] [PATCH v3 24/25] chardev: Let qemu_chr_fe_write[_all] use size_t type argument

[Marc-André Lureau]

On Wed, Feb 20, 2019 at 2:08 AM Philippe Mathieu-Daudé
<address@hidden> wrote:
>
> All caller have been audited and call these functions with
> unsigned arguments.
>
> Most of them use a size_t argument, or directly pass sizeof().
>
> One case is unclear: the mux_chr_write() call in chardev/char-mux.c.
> There we add an assert (which will be removed in few patches) and
> cast the parameter as size_t to make explicit this value is unsigned.

mux_chr_write() is called indirectly from qemu_chr_fe_write(), so the
same argument applies here. Fine with or without the assert().

>
> Suggested-by: Paolo Bonzini <address@hidden>
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>

Reviewed-by: Marc-André Lureau <address@hidden>

> ---
>  chardev/char-fe.c         | 4 ++--
>  chardev/char-mux.c        | 3 ++-
>  include/chardev/char-fe.h | 4 ++--
>  3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/chardev/char-fe.c b/chardev/char-fe.c
> index f3530a90e6..ab2a01709d 100644
> --- a/chardev/char-fe.c
> +++ b/chardev/char-fe.c
> @@ -31,7 +31,7 @@
>  #include "chardev/char-io.h"
>  #include "chardev/char-mux.h"
>
> -int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len)
> +int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, size_t len)
>  {
>      Chardev *s = be->chr;
>
> @@ -42,7 +42,7 @@ int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, 
> int len)
>      return qemu_chr_write(s, buf, len, false);
>  }
>
> -int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, int len)
> +int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, size_t len)
>  {
>      Chardev *s = be->chr;
>
> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
> index 23aa82125d..7a3ff21db4 100644
> --- a/chardev/char-mux.c
> +++ b/chardev/char-mux.c
> @@ -38,7 +38,8 @@ static int mux_chr_write(Chardev *chr, const uint8_t *buf, 
> int len)
>      MuxChardev *d = MUX_CHARDEV(chr);
>      int ret;
>      if (!d->timestamps) {
> -        ret = qemu_chr_fe_write(&d->chr, buf, len);
> +        assert(len >= 0);
> +        ret = qemu_chr_fe_write(&d->chr, buf, (size_t)len);
>      } else {
>          int i;
>
> diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
> index aa1b864ccd..5fb2c2e7ec 100644
> --- a/include/chardev/char-fe.h
> +++ b/include/chardev/char-fe.h
> @@ -203,7 +203,7 @@ guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition 
> cond,
>   *
>   * Returns: the number of bytes consumed (0 if no associated Chardev)
>   */
> -int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len);
> +int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, size_t len);
>
>  /**
>   * qemu_chr_fe_write_all:
> @@ -217,7 +217,7 @@ int qemu_chr_fe_write(CharBackend *be, const uint8_t 
> *buf, int len);
>   *
>   * Returns: the number of bytes consumed (0 if no associated Chardev)
>   */
> -int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, int len);
> +int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, size_t len);
>
>  /**
>   * qemu_chr_fe_read_all:
> --
> 2.20.1
>