Re: [PATCH v2 4/5] vfio-user: simplify vfio_user_recv_one()

[Cédric Le Goater]

On 11/17/25 16:56, John Levon wrote:

This function was unnecessarily difficult to understand due to the
separate handling of request and reply messages. Use common code for
both where we can.

It's still difficult to read :) Could we have feedback from Thanos or Mark ?

Thanks,

C.

Signed-off-by: John Levon <john.le...@nutanix.com>
---
  hw/vfio-user/proxy.c | 68 +++++++++++++++++++-------------------------
  1 file changed, 30 insertions(+), 38 deletions(-)

diff --git a/hw/vfio-user/proxy.c b/hw/vfio-user/proxy.c
index 87e50501af..aa5b971fb6 100644
--- a/hw/vfio-user/proxy.c
+++ b/hw/vfio-user/proxy.c
@@ -281,15 +281,14 @@ static int vfio_user_recv_hdr(VFIOUserProxy *proxy, Error 
**errp,
   */
  static int vfio_user_recv_one(VFIOUserProxy *proxy, Error **errp)
  {
-    VFIOUserMsg *msg = NULL;
      g_autofree int *fdp = NULL;
-    VFIOUserFDs *reqfds;
-    VFIOUserHdr hdr;
+    VFIOUserMsg *msg = NULL;
      bool isreply = false;
-    int i, ret;
-    size_t msgleft, numfds = 0;
+    size_t msgleft = 0;
+    size_t numfds = 0;
      char *data = NULL;
-    char *buf = NULL;
+    VFIOUserHdr hdr;
+    int i, ret;

/*

       * Complete any partial reads
@@ -317,8 +316,8 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error 
**errp)
      }

/*

-     * For replies, find the matching pending request.
-     * For requests, reap incoming FDs.
+     * Find the matching request if this is a reply, or initialize a new
+     * server->client request.
       */
      if (isreply) {
          QTAILQ_FOREACH(msg, &proxy->pending, next) {
@@ -332,51 +331,44 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error 
**errp)
          }
          QTAILQ_REMOVE(&proxy->pending, msg, next);

- /*

-         * Process any received FDs
-         */
-        if (numfds != 0) {
-            if (msg->fds == NULL || msg->fds->recv_fds < numfds) {
-                error_setg(errp, "unexpected FDs");
-                goto err;
-            }
-            msg->fds->recv_fds = numfds;
-            memcpy(msg->fds->fds, fdp, numfds * sizeof(int));
-        }
-    } else {
-        if (numfds != 0) {
-            reqfds = vfio_user_getfds(numfds);
-            memcpy(reqfds->fds, fdp, numfds * sizeof(int));
-        } else {
-            reqfds = NULL;
-        }
-    }
-
-    /*
-     * Put the whole message into a single buffer.
-     */
-    if (isreply) {
          if (hdr.size > msg->rsize) {
              error_setg(errp, "reply larger than recv buffer");
              goto err;
          }
-        *msg->hdr = hdr;
-        data = (char *)msg->hdr + sizeof(hdr);
      } else {
+        void *buf;
+
          if (hdr.size > proxy->max_xfer_size + sizeof(VFIOUserDMARW)) {
              error_setg(errp, "vfio_user_recv request larger than max");
              goto err;
          }
+
          buf = g_malloc0(hdr.size);
-        memcpy(buf, &hdr, sizeof(hdr));
-        data = buf + sizeof(hdr);
-        msg = vfio_user_getmsg(proxy, (VFIOUserHdr *)buf, reqfds);
+        msg = vfio_user_getmsg(proxy, buf, NULL);
          msg->type = VFIO_MSG_REQ;
      }

+ *msg->hdr = hdr;

+    data = (char *)msg->hdr + sizeof(hdr);
+
+    if (numfds != 0) {
+        if (msg->type == VFIO_MSG_REQ) {
+            msg->fds = vfio_user_getfds(numfds);
+        } else {
+            if (msg->fds == NULL || msg->fds->recv_fds < numfds) {
+                error_setg(errp, "unexpected FDs");
+                goto err;
+            }
+            msg->fds->recv_fds = numfds;
+        }
+
+        memcpy(msg->fds->fds, fdp, numfds * sizeof(int));
+    }
+
      /*
-     * Read rest of message.
+     * Read rest of message into the data buffer.
       */
+
      msgleft = hdr.size - sizeof(hdr);
      while (msgleft > 0) {
          ret = qio_channel_read(proxy->ioc, data, msgleft, errp);