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<m...@tls.msk.ru>
>> ---
>> 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
