[Top][All Lists]

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

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

From: Arwed Meyer
Subject: Re: [PATCH v2 3/5] msmouse: Use fifo8 instead of array
Date: Sun, 11 Sep 2022 19:53:14 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0

Am 11.09.22 um 08:12 schrieb Volker Rümelin:
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)) {
-    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,


  static void msmouse_queue_event(MouseChardev *mouse)

Hi Volker,

have to admit I was not completely sure about the order in which
function arguments get evaluated either. Thanks for the confirmation.
I'll change this to be safe.

I guess you probably won't see much difference between old and new code
in practice since serial I/O is slow anyway, but since I'll change the
patch anyway, I'll take the code from gd_vc_send_chars. Indeed looks better.

reply via email to

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