> On 9 Feb 2018, at 17:59, Frediano Ziglio <fzig...@redhat.com> wrote:
>>>>> +
>>>>> +    // read part of the message till we get all
>>>>> +    if (dev->msg_len < dev->hdr.size) {
>>>>> +        dev->msg = g_realloc(dev->msg, dev->hdr.size);
>>>>> +        dev->msg_len = dev->hdr.size;
>>>>> +    }
>>>>> +    int n = sif->read(sin, dev->msg->buf + dev->msg_pos, dev->hdr.size -
>>>>> dev->msg_pos);
>>>>> +    if (n <= 0) {
>>>>> +        return false;
>>>>> +    }
>>>>> +    dev->msg_pos += n;
>>>>> +    if (dev->msg_pos != dev->hdr.size) {
>>>> Is it assumed you can all read in one go? I’m surprized there is a “while”
>>>> loop for reading the header (which is small) but no while loop for reading
>>>> the payload which is larger).
>>> When we return false is not an error, just there's no data left next time
>>> will add more data. So there's no while but there's a loop anyway.
>>> I suppose similarly we could avoid the while in the other function.
>> Oh, so you are saying that you think it’s OK to loop ouside, truncate the
>> data that you g_realloc’d, and then start over?
>> Does not work. Say you started with 1K in buffer, then go to this inner read
>> here, which does a g_realloc and reads 10K. Then we re-enter the above
>> function, so I believe we will truncate to 1K again, and then re-extend to
>> 10K and start reading at 10K. We lost 9K of data, unless I’m mistaken.
> No, the header is not read again until all message is processed.
> There are also tests for this.

This is not what I am talking about. I’m saying you read this, where H is 
header and D is data.

You first read header, let’s say four items.


Then you realloc, which puts some undefined garbage in the area the data:


Then you read data, get 4 elements:


So you restart in the outer loop, where you truncate the header


and reallocate


and restart reading, not sure if we have reset msg_pos or not, so you get either




None of which is correct. I’ve not seen in the code how we prevent this if we 
don’t loop in the inner read.

Maybe I missed the part that makes us restart correctly, but where did we keep 
the data?


Spice-devel mailing list

Reply via email to