[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv4 01/11] virtio-serial-bus: use correct lengths
From: |
Michael Tokarev |
Subject: |
Re: [Qemu-devel] [PATCHv4 01/11] virtio-serial-bus: use correct lengths in control_out() message |
Date: |
Fri, 16 Mar 2012 20:34:54 +0400 |
User-agent: |
Mozilla/5.0 (X11; Linux i686 on x86_64; rv:8.0) Gecko/20120216 Icedove/8.0 |
On 16.03.2012 20:12, Anthony Liguori wrote:
> On 03/15/2012 04:00 PM, Michael Tokarev wrote:
>> In case of more than one control message, the code will use
>> size of the largest message so far for all subsequent messages,
>> instead of using size of current one. Fix it.
>>
>> Signed-off-by: Michael Tokarev<address@hidden>
>> ---
>> hw/virtio-serial-bus.c | 6 +++---
>> 1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
>> index e22940e..abe48ec 100644
>> --- a/hw/virtio-serial-bus.c
>> +++ b/hw/virtio-serial-bus.c
>> @@ -454,7 +454,7 @@ static void control_out(VirtIODevice *vdev, VirtQueue
>> *vq)
>> len = 0;
>> buf = NULL;
>> while (virtqueue_pop(vq,&elem)) {
>> - size_t cur_len, copied;
>> + size_t cur_len;
>>
>> cur_len = iov_size(elem.out_sg, elem.out_num);
>> /*
>> @@ -467,9 +467,9 @@ static void control_out(VirtIODevice *vdev, VirtQueue
>> *vq)
>> buf = g_malloc(cur_len);
>> len = cur_len;
>> }
>> - copied = iov_to_buf(elem.out_sg, elem.out_num, buf, 0, len);
>> + iov_to_buf(elem.out_sg, elem.out_num, buf, 0, cur_len);
>
> I don't understand what this is fixing. copied = cur_len unless for some
> reason a full copy couldn't be done.
>
> But you're assuming a full copy is done. So I'm confused by what is being
> fixed here.
This is the same thing which the author of this code didn't
understand too, and the whole reason why I changed this code.
My answer:
http://thread.gmane.org/gmane.comp.emulators.qemu/140477/focus=140572
"We got one thing, we requested to copy another, and we handle
3rd which is something else. While actually we are supposed
to get, request and handle the _same_, or else we're doomed."
In the same thread:
http://thread.gmane.org/gmane.comp.emulators.qemu/140477/focus=140572
"It is like doing, for a memcpy-like function:
void *memdup(const void *src, size_t size) {
char *dest = malloc(size+1);
size_t copied = copybytes(dest, src, size+1);
if (copied != size+1) {
/* What?? */
}
return dest;
}
The only sane thing here, I think, is to drop 'copied',
to stop any possible confusion :)"
Thanks,
/mjt
- [Qemu-devel] [PATCHv4 00/11] cleanup/consolidate iovec functions, Michael Tokarev, 2012/03/15
- [Qemu-devel] [PATCHv4 09/11] export iov_send_recv() and use it in iov_send() and iov_recv(), Michael Tokarev, 2012/03/15
- [Qemu-devel] [PATCHv4 08/11] rename qemu_sendv to iov_send, change proto and move declarations to iov.h, Michael Tokarev, 2012/03/15
- [Qemu-devel] [PATCHv4 01/11] virtio-serial-bus: use correct lengths in control_out() message, Michael Tokarev, 2012/03/15
- [Qemu-devel] [PATCHv4 04/11] consolidate qemu_iovec_memset{, _skip}() into single function and use existing iov_memset(), Michael Tokarev, 2012/03/15
- Re: [Qemu-devel] [PATCHv4 04/11] consolidate qemu_iovec_memset{, _skip}() into single function and use existing iov_memset(), Anthony Liguori, 2012/03/16
- Re: [Qemu-devel] [PATCHv4 04/11] consolidate qemu_iovec_memset{, _skip}() into single function and use existing iov_memset(), Stefan Hajnoczi, 2012/03/19
- Re: [Qemu-devel] [PATCHv4 04/11] consolidate qemu_iovec_memset{, _skip}() into single function and use existing iov_memset(), Michael Tokarev, 2012/03/19
- Re: [Qemu-devel] [PATCHv4 04/11] consolidate qemu_iovec_memset{, _skip}() into single function and use existing iov_memset(), Anthony Liguori, 2012/03/19
- Re: [Qemu-devel] [PATCHv4 04/11] consolidate qemu_iovec_memset{, _skip}() into single function and use existing iov_memset(), Stefan Hajnoczi, 2012/03/20
[Qemu-devel] [PATCHv4 10/11] cleanup qemu_co_sendv(), qemu_co_recvv() and friends, Michael Tokarev, 2012/03/15
[Qemu-devel] [PATCHv4 06/11] consolidate qemu_iovec_copy() and qemu_iovec_concat() and make them consistent, Michael Tokarev, 2012/03/15