qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH v2 3/5] msmouse: Use fifo8 instead of array


From: Volker Rümelin
Subject: Re: [PATCH v2 3/5] msmouse: Use fifo8 instead of array
Date: Sun, 11 Sep 2022 08:12:26 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0

Am 08.09.22 um 19:31 schrieb Arwed Meyer:

@@ -54,21 +60,15 @@ DECLARE_INSTANCE_CHECKER(MouseChardev, MOUSE_CHARDEV,
  static void msmouse_chr_accept_input(Chardev *chr)
  {
      MouseChardev *mouse = MOUSE_CHARDEV(chr);
-    int len;
+    uint32_t len_out, len;

-    len = qemu_chr_be_can_write(chr);
-    if (len > mouse->outlen) {
-        len = mouse->outlen;
-    }
-    if (!len) {
+    len_out = qemu_chr_be_can_write(chr);
+    if (!len_out || fifo8_is_empty(&mouse->outbuf)) {
          return;
      }
-
-    qemu_chr_be_write(chr, mouse->outbuf, len);
-    mouse->outlen -= len;
-    if (mouse->outlen) {
-        memmove(mouse->outbuf, mouse->outbuf + len, mouse->outlen);
-    }
+    len = MIN(fifo8_num_used(&mouse->outbuf), len_out);
+    qemu_chr_be_write(chr, fifo8_pop_buf(&mouse->outbuf, len, &len_out),
+            len_out);

Hi Arwed,

I think C function arguments are not evaluated in a defined order. It's not defined if the third argument of function qemu_chr_be_write() is len_out before or after the call to fifo8_pop_buf().

The fifo_pop_buf() function uses a ringbuffer. When the buffer wraps around at the end and the ringbuffer contains more than one byte you may need two fifo8_pop_buf() and qemu_chr_be_write() calls to write all bytes. The code you replace doesn't have that problem.

Some chardev frontends don't return the total number of bytes to write in qemu_chr_be_can_write(). They return the number of bytes that can be written with one qemu_chr_be_write() call. You need another qemu_chr_be_can_write() call after the qemu_chr_be_write() call to see if more bytes can be written.

The code in function gd_vc_send_chars() in ui/gtk.c could be used as a template to avoid the three issues above.

With best regards,
Volker

  }

  static void msmouse_queue_event(MouseChardev *mouse)




reply via email to

[Prev in Thread] Current Thread [Next in Thread]