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

[Marc-André Lureau]

Hi

On Thu, Sep 8, 2022 at 9:38 PM Arwed Meyer <arwed.meyer@gmx.de> wrote:

Make use of fifo8 functions instead of implementing own fifo code.
This makes the code more readable and reduces risk of bugs.

Signed-off-by: Arwed Meyer <arwed.meyer@gmx.de>

 

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

 

---
 chardev/msmouse.c | 43 +++++++++++++++++++++----------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/chardev/msmouse.c b/chardev/msmouse.c
index 95fa488339..e9aa3eab55 100644
--- a/chardev/msmouse.c
+++ b/chardev/msmouse.c
@@ -24,6 +24,7 @@

 #include "qemu/osdep.h"
 #include "qemu/module.h"
+#include "qemu/fifo8.h"
 #include "chardev/char.h"
 #include "chardev/char-serial.h"
 #include "ui/console.h"
@@ -34,6 +35,12 @@
 #define MSMOUSE_HI2(n)  (((n) & 0xc0) >> 6)
 #define MSMOUSE_PWR(cm) (cm & (CHR_TIOCM_RTS | CHR_TIOCM_DTR))

+/* Serial fifo size. */
+#define MSMOUSE_BUF_SZ 64

Why bump to 64 bytes?

 

+
+/* Mouse ID: Send "M3" cause we behave like a 3 button logitech mouse. */
+const uint8_t mouse_id[] = {'M', '3'};
+
 struct MouseChardev {
     Chardev parent;

@@ -42,8 +49,7 @@ struct MouseChardev {
     int axis[INPUT_AXIS__MAX];
     bool btns[INPUT_BUTTON__MAX];
     bool btnc[INPUT_BUTTON__MAX];
-    uint8_t outbuf[32];
-    int outlen;
+    Fifo8 outbuf;
 };
 typedef struct MouseChardev MouseChardev;

@@ -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);
 }

 static void msmouse_queue_event(MouseChardev *mouse)
@@ -94,12 +94,11 @@ static void msmouse_queue_event(MouseChardev *mouse)
         mouse->btnc[INPUT_BUTTON_MIDDLE]) {
         bytes[3] |= (mouse->btns[INPUT_BUTTON_MIDDLE] ? 0x20 : 0x00);
         mouse->btnc[INPUT_BUTTON_MIDDLE] = false;
-        count = 4;
+        count++;

a bit superfluous,  ok

     }

-    if (mouse->outlen <= sizeof(mouse->outbuf) - count) {
-        memcpy(mouse->outbuf + mouse->outlen, bytes, count);
-        mouse->outlen += count;
+    if (fifo8_num_free(&mouse->outbuf) >= count) {
+        fifo8_push_all(&mouse->outbuf, bytes, count);
     } else {
         /* queue full -> drop event */
     }
@@ -172,9 +171,7 @@ static int msmouse_ioctl(Chardev *chr, int cmd, void *arg)
                  * cause we behave like a 3 button logitech
                  * mouse.
                  */
-                mouse->outbuf[0] = 'M';
-                mouse->outbuf[1] = '3';
-                mouse->outlen = 2;
+                fifo8_push_all(&mouse->outbuf, mouse_id, sizeof(mouse_id));
                 /* Start sending data to serial. */
                 msmouse_chr_accept_input(chr);
             }
@@ -184,7 +181,7 @@ static int msmouse_ioctl(Chardev *chr, int cmd, void *arg)
          * Reset mouse buffers on power down.
          * Mouse won't send anything without power.
          */
-        mouse->outlen = 0;
+        fifo8_reset(&mouse->outbuf);
         memset(mouse->axis, 0, sizeof(mouse->axis));
         memset(mouse->btns, false, sizeof(mouse->btns));
         memset(mouse->btnc, false, sizeof(mouse->btns));
@@ -204,6 +201,7 @@ static void char_msmouse_finalize(Object *obj)
     MouseChardev *mouse = MOUSE_CHARDEV(obj);

     qemu_input_handler_unregister(mouse->hs);
+    fifo8_destroy(&mouse->outbuf);
 }

 static QemuInputHandler msmouse_handler = {
@@ -224,6 +222,7 @@ static void msmouse_chr_open(Chardev *chr,
     mouse->hs = qemu_input_handler_register((DeviceState *)mouse,
                                             &msmouse_handler);
     mouse->tiocm = 0;
+    fifo8_create(&mouse->outbuf, MSMOUSE_BUF_SZ);
 }

 static void char_msmouse_class_init(ObjectClass *oc, void *data)
--
2.34.1

--

Marc-André Lureau