[Qemu-devel] [PATCH 2/3] block/vdi: Don't take address of fields in packed structs

[Peter Maydell <peter.maydell@linaro.org>]

Taking the address of a field in a packed struct is a bad idea, because
it might not be actually aligned enough for that pointer type (and
thus cause a crash on dereference on some host architectures). Newer
versions of clang warn about this.

Instead of passing UUID related functions the address of a possibly
unaligned QemuUUID struct, use local variables and then copy to/from
the struct field as appropriate.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 block/vdi.c | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/block/vdi.c b/block/vdi.c
index 2380daa583e..4cc726047c3 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -235,7 +235,8 @@ static void vdi_header_to_le(VdiHeader *header)
 
 static void vdi_header_print(VdiHeader *header)
 {
-    char uuid[37];
+    char uuidstr[37];
+    QemuUUID uuid;
     logout("text        %s", header->text);
     logout("signature   0x%08x\n", header->signature);
     logout("header size 0x%04x\n", header->header_size);
@@ -254,14 +255,18 @@ static void vdi_header_print(VdiHeader *header)
     logout("block extra 0x%04x\n", header->block_extra);
     logout("blocks tot. 0x%04x\n", header->blocks_in_image);
     logout("blocks all. 0x%04x\n", header->blocks_allocated);
-    qemu_uuid_unparse(&header->uuid_image, uuid);
-    logout("uuid image  %s\n", uuid);
-    qemu_uuid_unparse(&header->uuid_last_snap, uuid);
-    logout("uuid snap   %s\n", uuid);
-    qemu_uuid_unparse(&header->uuid_link, uuid);
-    logout("uuid link   %s\n", uuid);
-    qemu_uuid_unparse(&header->uuid_parent, uuid);
-    logout("uuid parent %s\n", uuid);
+    uuid = header->uuid_image;
+    qemu_uuid_unparse(&uuid, uuidstr);
+    logout("uuid image  %s\n", uuidstr);
+    uuid = header->uuid_last_snap;
+    qemu_uuid_unparse(&uuid, uuidstr);
+    logout("uuid snap   %s\n", uuidstr);
+    uuid = header->uuid_link;
+    qemu_uuid_unparse(&uuid, uuidstr);
+    logout("uuid link   %s\n", uuidstr);
+    uuid = header->uuid_parent;
+    qemu_uuid_unparse(&uuid, uuidstr);
+    logout("uuid parent %s\n", uuidstr);
 }
 
 static int coroutine_fn vdi_co_check(BlockDriverState *bs, BdrvCheckResult *res,
@@ -368,6 +373,7 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
     size_t bmap_size;
     int ret;
     Error *local_err = NULL;
+    QemuUUID uuid_link, uuid_parent;
 
     bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
                                false, errp);
@@ -395,6 +401,9 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
+    uuid_link = header.uuid_link;
+    uuid_parent = header.uuid_parent;
+
     if (header.disk_size % SECTOR_SIZE != 0) {
         /* 'VBoxManage convertfromraw' can create images with odd disk sizes.
            We accept them but round the disk size to the next multiple of
@@ -444,11 +453,11 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
                    (uint64_t)header.blocks_in_image * header.block_size);
         ret = -ENOTSUP;
         goto fail;
-    } else if (!qemu_uuid_is_null(&header.uuid_link)) {
+    } else if (!qemu_uuid_is_null(&uuid_link)) {
         error_setg(errp, "unsupported VDI image (non-NULL link UUID)");
         ret = -ENOTSUP;
         goto fail;
-    } else if (!qemu_uuid_is_null(&header.uuid_parent)) {
+    } else if (!qemu_uuid_is_null(&uuid_parent)) {
         error_setg(errp, "unsupported VDI image (non-NULL parent UUID)");
         ret = -ENOTSUP;
         goto fail;
@@ -733,6 +742,7 @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options,
     BlockDriverState *bs_file = NULL;
     BlockBackend *blk = NULL;
     uint32_t *bmap = NULL;
+    QemuUUID uuid;
 
     assert(create_options->driver == BLOCKDEV_DRIVER_VDI);
     vdi_opts = &create_options->u.vdi;
@@ -819,8 +829,10 @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options,
     if (image_type == VDI_TYPE_STATIC) {
         header.blocks_allocated = blocks;
     }
-    qemu_uuid_generate(&header.uuid_image);
-    qemu_uuid_generate(&header.uuid_last_snap);
+    qemu_uuid_generate(&uuid);
+    header.uuid_image = uuid;
+    qemu_uuid_generate(&uuid);
+    header.uuid_last_snap = uuid;
     /* There is no need to set header.uuid_link or header.uuid_parent here. */
     if (VDI_DEBUG) {
         vdi_header_print(&header);
-- 
2.19.2