Re: [PATCH] vhost-user: rewrite vu_dispatch with if-else

[Stefano Garzarella]

On Sun, Aug 04, 2024 at 10:23:53PM GMT, luzhixing12345 wrote:

rewrite with if-else instead of goto

Why?

IMHO was better before this patch with a single error path.

and I have a question, in two incorrent cases

- need reply but no reply_requested
- no need reply but has reply_requested

should we call vu_panic or print warning message?

---
subprojects/libvhost-user/libvhost-user.c | 39 +++++++++++++----------
subprojects/libvhost-user/libvhost-user.h |  6 ++--
2 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c

index 9c630c2170..187e25f9bb 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -2158,32 +2158,39 @@ vu_dispatch(VuDev *dev)
{
    VhostUserMsg vmsg = { 0, };
    int reply_requested;
-    bool need_reply, success = false;
+    bool need_reply, success = true;

    if (!dev->read_msg(dev, dev->sock, &vmsg)) {
-        goto end;
+        success = false;
+        free(vmsg.data);
+        return success;
    }

    need_reply = vmsg.flags & VHOST_USER_NEED_REPLY_MASK;

    reply_requested = vu_process_message(dev, &vmsg);
-    if (!reply_requested && need_reply) {
-        vmsg_set_reply_u64(&vmsg, 0);
-        reply_requested = 1;
-    }
-
-    if (!reply_requested) {
-        success = true;
-        goto end;
-    }

-    if (!vu_send_reply(dev, dev->sock, &vmsg)) {
-        goto end;
+    if (need_reply) {
+        if (reply_requested) {
+            if (!vu_send_reply(dev, dev->sock, &vmsg)) {
+                success = false;
+            }
+        } else {
+            // need reply but no reply requested, return 0(u64)
+            vmsg_set_reply_u64(&vmsg, 0);
+            if (!vu_send_reply(dev, dev->sock, &vmsg)) {
+                success = false;
+            }
+        }
+    } else {
+        // no need reply but reply requested, send a reply
+        if (reply_requested) {
+            if (!vu_send_reply(dev, dev->sock, &vmsg)) {
+                success = false;
+            }
+        }
    }

-    success = true;
-
-end:
    free(vmsg.data);
    return success;
}
diff --git a/subprojects/libvhost-user/libvhost-user.h 
b/subprojects/libvhost-user/libvhost-user.h
index deb40e77b3..2daf8578f6 100644
--- a/subprojects/libvhost-user/libvhost-user.h
+++ b/subprojects/libvhost-user/libvhost-user.h
@@ -238,6 +238,8 @@ typedef struct VuDev VuDev;

typedef uint64_t (*vu_get_features_cb) (VuDev *dev);
typedef void (*vu_set_features_cb) (VuDev *dev, uint64_t features);
+typedef uint64_t (*vu_get_protocol_features_cb) (VuDev *dev);
+typedef void (*vu_set_protocol_features_cb) (VuDev *dev, uint64_t features);

Are these changes related?

Stefano

typedef int (*vu_process_msg_cb) (VuDev *dev, VhostUserMsg *vmsg,
                                  int *do_reply);
typedef bool (*vu_read_msg_cb) (VuDev *dev, int sock, VhostUserMsg *vmsg);
@@ -256,9 +258,9 @@ typedef struct VuDevIface {
    vu_set_features_cb set_features;
    /* get the protocol feature bitmask from the underlying vhost
     * implementation */
-    vu_get_features_cb get_protocol_features;
+    vu_get_protocol_features_cb get_protocol_features;
    /* enable protocol features in the underlying vhost implementation. */
-    vu_set_features_cb set_protocol_features;
+    vu_set_protocol_features_cb set_protocol_features;
    /* process_msg is called for each vhost-user message received */
    /* skip libvhost-user processing if return value != 0 */
    vu_process_msg_cb process_msg;
--
2.34.1