Re: [Qemu-block] [PATCH 0/3] block/qcow*: Don't take address of fields in packed structs

2018-11-05 Thread Peter Maydell
On 10 October 2018 at 11:55, Kevin Wolf  wrote:
> Am 09.10.2018 um 19:24 hat Peter Maydell geschrieben:
>> 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 in the affected files where the in-place swap
>> function is used on something other than a packed struct field; we
>> convert those anyway, for consistency.
>>
>> Patches produced mechanically using spatch; in one case I also
>> did a little hand-editing to wrap overlong lines that checkpatch
>> would otherwise complain about.
>>
>> (clang also complains about other files in block: vdi.c, vpc.c,
>> vhdx.h, vhdx.c, vhdx-endian.c, vhdx-log.c -- I may produce patches
>> for those later if nobody else gets there first.)
>>
>> thanks
>> -- PMM
>>
>> Peter Maydell (3):
>>   block/qcow2: Don't take address of fields in packed structs
>>   block/qcow: Don't take address of fields in packed structs
>>   block/qcow2-bitmap: Don't take address of fields in packed structs
>
> Thanks, applied to the block branch.

Ping? This doesn't seem to have made it into master, unless
I've missed it...

thanks
-- PMM



Re: [Qemu-block] [PATCH 0/3] block/qcow*: Don't take address of fields in packed structs

2018-10-10 Thread Kevin Wolf
Am 09.10.2018 um 19:24 hat Peter Maydell geschrieben:
> 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 in the affected files where the in-place swap
> function is used on something other than a packed struct field; we
> convert those anyway, for consistency.
> 
> Patches produced mechanically using spatch; in one case I also
> did a little hand-editing to wrap overlong lines that checkpatch
> would otherwise complain about.
> 
> (clang also complains about other files in block: vdi.c, vpc.c,
> vhdx.h, vhdx.c, vhdx-endian.c, vhdx-log.c -- I may produce patches
> for those later if nobody else gets there first.)
> 
> thanks
> -- PMM
> 
> Peter Maydell (3):
>   block/qcow2: Don't take address of fields in packed structs
>   block/qcow: Don't take address of fields in packed structs
>   block/qcow2-bitmap: Don't take address of fields in packed structs

Thanks, applied to the block branch.

Kevin



Re: [Qemu-block] [PATCH 0/3] block/qcow*: Don't take address of fields in packed structs

2018-10-09 Thread John Snow



On 10/09/2018 01:24 PM, 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 in the affected files where the in-place swap
> function is used on something other than a packed struct field; we
> convert those anyway, for consistency.
> 
> Patches produced mechanically using spatch; in one case I also
> did a little hand-editing to wrap overlong lines that checkpatch
> would otherwise complain about.
> 
> (clang also complains about other files in block: vdi.c, vpc.c,
> vhdx.h, vhdx.c, vhdx-endian.c, vhdx-log.c -- I may produce patches
> for those later if nobody else gets there first.)
> 
> thanks
> -- PMM
> 
> Peter Maydell (3):
>   block/qcow2: Don't take address of fields in packed structs
>   block/qcow: Don't take address of fields in packed structs
>   block/qcow2-bitmap: Don't take address of fields in packed structs
> 
>  block/qcow.c | 18 ++---
>  block/qcow2-bitmap.c | 24 -
>  block/qcow2.c| 64 +++-
>  3 files changed, 55 insertions(+), 51 deletions(-)
> 

Tested-by: John Snow 
Reviewed-by: John Snow 



[Qemu-block] [PATCH 0/3] block/qcow*: Don't take address of fields in packed structs

2018-10-09 Thread Peter Maydell
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 in the affected files where the in-place swap
function is used on something other than a packed struct field; we
convert those anyway, for consistency.

Patches produced mechanically using spatch; in one case I also
did a little hand-editing to wrap overlong lines that checkpatch
would otherwise complain about.

(clang also complains about other files in block: vdi.c, vpc.c,
vhdx.h, vhdx.c, vhdx-endian.c, vhdx-log.c -- I may produce patches
for those later if nobody else gets there first.)

thanks
-- PMM

Peter Maydell (3):
  block/qcow2: Don't take address of fields in packed structs
  block/qcow: Don't take address of fields in packed structs
  block/qcow2-bitmap: Don't take address of fields in packed structs

 block/qcow.c | 18 ++---
 block/qcow2-bitmap.c | 24 -
 block/qcow2.c| 64 +++-
 3 files changed, 55 insertions(+), 51 deletions(-)

-- 
2.19.0