qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Qemu-block] [PATCH] block/vdi: Don't take address of f


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH] block/vdi: Don't take address of fields in packed structs
Date: Wed, 17 Oct 2018 10:35:35 +0100
User-agent: Mutt/1.10.1 (2018-07-13)

On Tue, Oct 16, 2018 at 06:25:03PM +0100, Peter Maydell wrote:
> 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. Avoid the bug by not using the
> "modify in place" byte swapping functions.
> 
> There are a few places where the in-place swap function is
> used on something other than a packed struct field; we convert
> those anyway, for consistency.
> 
> Patch produced with scripts/coccinelle/inplace-byteswaps.cocci.
> 
> There are other places where we take the address of a packed member
> in this file for other purposes than passing it to a byteswap
> function (all the calls to qemu_uuid_*()); we leave those for now.
> 
> Signed-off-by: Peter Maydell <address@hidden>
> ---
> Another "tested with make check" auto-conversion patch. In this
> case, as noted above, it doesn't fix all the warnings for the file,
> but we might as well put the easy part of the fix in. I'm not sure
> what to do with the qemu_uuid_*() calls. Something like
>      QemuUUID uuid_link = header->uuid_link;
> and then using "qemu_uuid_is_null(uuid_link)" rather than

I would take this route.  (I think you mean qemu_uuid_is_null(&uuid_link).)

Reviewed-by: Stefan Hajnoczi <address@hidden>

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]