Re: [Qemu-devel] [PATCH v4 2/8] Add socket_writev_buffer function

[Orit Wasserman <owasserm@redhat.com>]

On 03/22/2013 07:08 PM, Eric Blake wrote:
> On 03/22/2013 01:30 AM, Orit Wasserman wrote:
> 
>>>>  
>>>> +static int socket_writev_buffer(void *opaque, struct iovec *iov, int iovcnt)
>>>
>>> Returning int...
>>>
>>>> +{
>>>> +    QEMUFileSocket *s = opaque;
>>>> +    ssize_t len;
>>>> +    ssize_t size = iov_size(iov, iovcnt);
>>>> +
>>>> +    len = iov_send(s->fd, iov, iovcnt, 0, size);
>>>> +    if (len < size) {
>>>> +        len = -socket_error();
>>>> +    }
>>>> +    return len;
>>>
>>> ...but len is an ssize_t.  If we send an iov with 2 gigabytes of data,
>>> this can wrap around to a negative int even though we send a positive
>>> amount of data.  Why not make the callback be typed to return ssize_t
>>> from the beginning (affects patch 1/8)?
>> At the moment it is not an issue but for the future we need to switch to ssize_t 
>> instead on int, I will change it.
>> We actually need to replace it all around the migration code but this should
>> be done in a different patch series.
> 
> I agree that the existing code base is in horrible shape with regards to
> int instead of ssize_t, and that it will take a different patch series
> to clean that up.  But why make that future patch harder?  New
> interfaces might as well be designed correctly, to limit the cleanup to
> the old interfaces, instead of making the cleanup job even harder.
> 
I agree completely! new interface should be designed correctly.