Replace protocol header flags bit field with mask (#773)

It turns out that the bit field will not yield the desired / specified bit layout on big-endian systems, see issue #768 for details. Thus, replace the bit field with constants for the individual fields and use bit masking when accessing the flags field. Signed-off-by: Mattias Nissler <[email protected]> Reviewed-by: John Levon <[email protected]> Reviewed-by: Thanos Makatos <[email protected]>
nutanix · Aug 30, 2023 · f981913 · f981913
1 parent 149aa84
commit f981913
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 23 deletions.
diff --git a/include/vfio-user.h b/include/vfio-user.h
@@ -81,14 +81,12 @@ struct vfio_user_header {
     uint16_t    msg_id;
     uint16_t    cmd;
     uint32_t    msg_size;
-    struct {
-        uint32_t    type     : 4;
-#define VFIO_USER_F_TYPE_COMMAND    0
-#define VFIO_USER_F_TYPE_REPLY      1
-        uint32_t    no_reply : 1;
-        uint32_t    error    : 1;
-        uint32_t    resvd    : 26;
-    } flags;
+    uint32_t    flags;
+#define VFIO_USER_F_TYPE_MASK       (0xf)
+#define VFIO_USER_F_TYPE_COMMAND    (0x0)
+#define VFIO_USER_F_TYPE_REPLY      (0x1)
+#define VFIO_USER_F_NO_REPLY        (0x10)
+#define VFIO_USER_F_ERROR           (0x20)
     uint32_t    error_no;
 } __attribute__((packed));
 

diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c
@@ -1127,7 +1127,7 @@ do_reply(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg, int reply_errno)
     assert(vfu_ctx != NULL);
     assert(msg != NULL);
 
-    if (msg->hdr.flags.no_reply) {
+    if (msg->hdr.flags & VFIO_USER_F_NO_REPLY) {
         /*
          * A failed client request is not a failure of handle_request() itself.
          */
@@ -1283,7 +1283,7 @@ get_request_header(vfu_ctx_t *vfu_ctx, vfu_msg_t **msgp)
 static bool
 is_valid_header(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg)
 {
-    if (msg->hdr.flags.type != VFIO_USER_F_TYPE_COMMAND) {
+    if ((msg->hdr.flags & VFIO_USER_F_TYPE_MASK) != VFIO_USER_F_TYPE_COMMAND) {
         vfu_log(vfu_ctx, LOG_ERR, "msg%#hx: not a command req",
                 msg->hdr.msg_id);
         return false;

diff --git a/lib/tran_pipe.c b/lib/tran_pipe.c
@@ -57,15 +57,15 @@ tran_pipe_send_iovec(int fd, uint16_t msg_id, bool is_reply,
     }
 
     if (is_reply) {
-        hdr.flags.type = VFIO_USER_F_TYPE_REPLY;
+        hdr.flags |= VFIO_USER_F_TYPE_REPLY;
         hdr.cmd = cmd;
         if (err != 0) {
-            hdr.flags.error = 1U;
+            hdr.flags |= VFIO_USER_F_ERROR;
             hdr.error_no = err;
         }
     } else {
         hdr.cmd = cmd;
-        hdr.flags.type = VFIO_USER_F_TYPE_COMMAND;
+        hdr.flags |= VFIO_USER_F_TYPE_COMMAND;
     }
 
     iovecs[0].iov_base = &hdr;
@@ -131,18 +131,18 @@ tran_pipe_recv(int fd, struct vfio_user_header *hdr, bool is_reply,
             return ERROR_INT(EPROTO);
         }
 
-        if (hdr->flags.type != VFIO_USER_F_TYPE_REPLY) {
+        if ((hdr->flags & VFIO_USER_F_TYPE_MASK) != VFIO_USER_F_TYPE_REPLY) {
             return ERROR_INT(EINVAL);
         }
 
-        if (hdr->flags.error == 1U) {
+        if (hdr->flags & VFIO_USER_F_ERROR) {
             if (hdr->error_no <= 0) {
                 hdr->error_no = EINVAL;
             }
             return ERROR_INT(hdr->error_no);
         }
     } else {
-        if (hdr->flags.type != VFIO_USER_F_TYPE_COMMAND) {
+        if ((hdr->flags & VFIO_USER_F_TYPE_MASK) != VFIO_USER_F_TYPE_COMMAND) {
             return ERROR_INT(EINVAL);
         }
         if (msg_id != NULL) {

diff --git a/lib/tran_sock.c b/lib/tran_sock.c
@@ -69,15 +69,15 @@ tran_sock_send_iovec(int sock, uint16_t msg_id, bool is_reply,
     memset(&msg, 0, sizeof(msg));
 
     if (is_reply) {
-        hdr.flags.type = VFIO_USER_F_TYPE_REPLY;
+        hdr.flags |= VFIO_USER_F_TYPE_REPLY;
         hdr.cmd = cmd;
         if (err != 0) {
-            hdr.flags.error = 1U;
+            hdr.flags |= VFIO_USER_F_ERROR;
             hdr.error_no = err;
         }
     } else {
         hdr.cmd = cmd;
-        hdr.flags.type = VFIO_USER_F_TYPE_COMMAND;
+        hdr.flags |= VFIO_USER_F_TYPE_COMMAND;
     }
 
     iovecs[0].iov_base = &hdr;
@@ -211,18 +211,18 @@ tran_sock_recv_fds(int sock, struct vfio_user_header *hdr, bool is_reply,
             return ERROR_INT(EPROTO);
         }
 
-        if (hdr->flags.type != VFIO_USER_F_TYPE_REPLY) {
+        if ((hdr->flags & VFIO_USER_F_TYPE_MASK) != VFIO_USER_F_TYPE_REPLY) {
             return ERROR_INT(EINVAL);
         }
 
-        if (hdr->flags.error == 1U) {
+        if (hdr->flags & VFIO_USER_F_ERROR) {
             if (hdr->error_no <= 0) {
                 hdr->error_no = EINVAL;
             }
             return ERROR_INT(hdr->error_no);
         }
     } else {
-        if (hdr->flags.type != VFIO_USER_F_TYPE_COMMAND) {
+        if ((hdr->flags & VFIO_USER_F_TYPE_MASK) != VFIO_USER_F_TYPE_COMMAND) {
             return ERROR_INT(EINVAL);
         }
         if (msg_id != NULL) {

diff --git a/test/unit-tests.c b/test/unit-tests.c
@@ -105,7 +105,7 @@ setup(void **state UNUSED)
 
     memset(&msg, 0, sizeof(msg));
 
-    msg.hdr.flags.type = VFIO_USER_F_TYPE_COMMAND;
+    msg.hdr.flags |= VFIO_USER_F_TYPE_COMMAND;
     msg.hdr.msg_size = sizeof(msg.hdr);
 
     fds[0] = fds[1] = -1;