[Spice-devel] [xf86-video-qxl v5] Enable smartcard support for XSpice.

[Uri Lublin]

On 12/12/2014 09:39 PM, Jeremy White wrote:
> On 12/12/2014 12:16 PM, Marc-André Lureau wrote:
>
>>> +    while (1) {
>>> +        rc = read(ccid->fd, buf + pos, sizeof(buf) - pos);
>>> +        if (rc == -1)
>>> +            if (errno == EINTR)
>>> +                continue;
>>> +            else
>>> +                break;
>>> +
>>> +        if (rc == 0)
>>> +            break;
>>> +
>>> +        pos += rc;
>>> +
>>> +        do {
>>> +            rc = process_message(ccid, buf, pos);
>>> +            pos -= rc;
>>
>> So you assume that messages are not sent "in batch" and you won't
>> override previous message? I guess you could assert() that pos == rc.
>> And then this loop is useless right? Or you need to increment buf?
>> hmm, I think you should rather memcpy the remaining bits if any
>>
>>> +        } while (rc > 0);
>>
>> I guess it could warn if pos == sizeof(buf) here
>
> Thanks for assuming the best of me, but it's just a bug on my part :-/.
>
> I believe adding this to the bottom of the loop:
>             if (rc > 0 && pos > 0)
>                 memmove(buf, buf + rc, pos);
>
> corrects it.
>
> I'll send a v6 in a bit; thanks for the review.

That looks correct to me, but it's maybe better to memmove outside the loop,
such that memmove is called once every read and not for every
message processed.
Maybe it does not matter as this was not noticed so far, so messages
mostly come one at a time.

size_t ppos = 0
do {
   rc = process_message(ccid, buf + ppos, pos - ppos);
   ppos += rc
} while (rc> 0)
if (ppos > 0 && ppos != pos)
   memmove(buf, buf+ppos, pos - ppos)

Thanks,
    Uri.