Re: [qemu-s390x] [Qemu-devel] [PATCH v3 25/25] chardev: Let qemu_chr_write[_all] use size_t

[Philippe Mathieu-Daudé]

On 2/20/19 11:42 AM, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Feb 20, 2019 at 11:39 AM Daniel P. Berrangé <address@hidden> wrote:
>>
>> On Wed, Feb 20, 2019 at 02:02:32AM +0100, Philippe Mathieu-Daudé wrote:
>>
>>> diff --git a/include/chardev/char.h b/include/chardev/char.h
>>> index 0341dd1ba2..2e3b5a15ca 100644
>>> --- a/include/chardev/char.h
>>> +++ b/include/chardev/char.h
>>> @@ -221,7 +221,7 @@ void qemu_chr_set_feature(Chardev *chr,
>>>                            ChardevFeature feature);
>>>  QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename,
>>>                                  bool permit_mux_mon);
>>> -int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool 
>>> write_all);
>>> +int qemu_chr_write(Chardev *s, const uint8_t *buf, size_t len, bool 
>>> write_all);
>>
>> Seeing this cleanup reminds me that I think we ought to change
>> the chardev read & write functions to take "void *buf" instead.
>> as is done for regular libc  read/write functions. This would
>> avoid casts in the callers between char */uint8_t *
>>
>> Something to think about for a future cleanup job....same applies
>> for the QIOChannel APIs which take a 'char *buf', annoyingly
>> different from the chardev APIs :-( Both ought to have void *buf
> 
> I fully agree, and I started that conversion some time ago, and lost
> it somewhere. That should be easier than changing the sign of return
> values though!

I thought about it too and am happy both of you agree.

The "part #3" update the prototypes to return ssize_t instead of int.
I can do this change there.

>>
>>
>> Regards,
>> Daniel
>> --
>> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange 
>> :|
>> |: https://libvirt.org         -o-            https://fstop138.berrange.com 
>> :|
>> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange 
>> :|