[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [qemu-s390x] [PATCH v3 00/25] chardev: Convert qemu_chr_write() to t
|
From: |
Cornelia Huck |
|
Subject: |
Re: [qemu-s390x] [PATCH v3 00/25] chardev: Convert qemu_chr_write() to take a size_t argument |
|
Date: |
Wed, 20 Feb 2019 11:57:49 +0100 |
On Wed, 20 Feb 2019 11:53:42 +0100
Marc-André Lureau <address@hidden> wrote:
> Hi
>
> On Wed, Feb 20, 2019 at 2:02 AM Philippe Mathieu-Daudé
> <address@hidden> wrote:
> >
> > Hi,
> >
> > This series convert the chardev::qemu_chr_write() to take unsigned
> > length argument. To do so I went through all caller and checked if
> > there are no negative value possible.
>
>
> Changing signedness is problematic and can easily introduce bugs that
> are easy to miss during review.
>
> I agree with Cornelia about idiomatic use of int. Changing "int" for
> "size_t" isn't systematically a clear win.
>
> Even Google C++ style recommends to avoid unsigned types "(except for
> representing bitfields or modular arithmetic). Do not use an unsigned
> type merely to assert that a variable is non-negative."
> https://google.github.io/styleguide/cppguide.html#Integer_Types - see
> rationale
>
> Since Paolo you suggested the change, could you give some convincing
> arguments that it's worth taking the plunge?
FWIW, using an explicitly unsigned type for a length sounds fine; but
not all conversions are really convincing (albeit not wrong).
- Re: [qemu-s390x] [PATCH v3 16/25] tpm: Use size_t to hold sizes, (continued)
- [qemu-s390x] [PATCH v3 18/25] s390x/3270: Let insert_IAC_escape_char() use size_t, Philippe Mathieu-Daudé, 2019/02/19
- [qemu-s390x] [PATCH v3 21/25] s390x/sclp: Use size_t in process_mdb(), Philippe Mathieu-Daudé, 2019/02/19
- [qemu-s390x] [PATCH v3 22/25] s390x/sclp: Let write_console_data() take a size_t, Philippe Mathieu-Daudé, 2019/02/19
- [qemu-s390x] [PATCH v3 24/25] chardev: Let qemu_chr_fe_write[_all] use size_t type argument, Philippe Mathieu-Daudé, 2019/02/19
- Re: [qemu-s390x] [PATCH v3 00/25] chardev: Convert qemu_chr_write() to take a size_t argument, Marc-André Lureau, 2019/02/20