From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:34849)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <owasserm@redhat.com>) id 1UJIWt-0005EH-T0
	for qemu-devel@nongnu.org; Sat, 23 Mar 2013 03:06:16 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <owasserm@redhat.com>) id 1UJIWs-0000KV-RT
	for qemu-devel@nongnu.org; Sat, 23 Mar 2013 03:06:15 -0400
Received: from mx1.redhat.com ([209.132.183.28]:45915)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <owasserm@redhat.com>) id 1UJIWs-0000KO-IK
	for qemu-devel@nongnu.org; Sat, 23 Mar 2013 03:06:14 -0400
Message-ID: <514D54A8.1050609@redhat.com>
Date: Sat, 23 Mar 2013 09:07:20 +0200
From: Orit Wasserman <owasserm@redhat.com>
MIME-Version: 1.0
References: <1363890878-8161-1-git-send-email-owasserm@redhat.com>
	<1363890878-8161-3-git-send-email-owasserm@redhat.com>
	<514B60E5.6090707@redhat.com> <514C0880.9000005@redhat.com>
	<514C9014.9030607@redhat.com>
In-Reply-To: <514C9014.9030607@redhat.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v4 2/8] Add socket_writev_buffer function
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Eric Blake <eblake@redhat.com>
Cc: pbonzini@redhat.com, quintela@redhat.com, chegu_vinod@hp.com, qemu-devel@nongnu.org, mst@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.