Re: [xf86-video-qxl v5] Enable smartcard support for XSpice.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]