[Qemu-devel] [PATCH 0/3] block: fix last address-of-packed-member warnings

Peter Maydell posted 3 patches 5 years, 4 months ago
Test checkpatch passed
Test docker-quick@centos7 passed
Test docker-clang@ubuntu passed
Test docker-mingw@fedora passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20181210112649.11581-1-peter.maydell@linaro.org
include/qemu/uuid.h  |  2 +-
block/vdi.c          | 54 +++++++++++++++++++++++++++-----------------
block/vpc.c          |  4 +++-
hw/acpi/vmgenid.c    |  6 ++---
tests/vmgenid-test.c |  2 +-
util/uuid.c          | 10 ++++----
6 files changed, 45 insertions(+), 33 deletions(-)
[Qemu-devel] [PATCH 0/3] block: fix last address-of-packed-member warnings
Posted by Peter Maydell 5 years, 4 months ago
This patchset fixes the remaining clang warnings in the block/ code
about taking the address of a packed struct member, which are all
in block/vpc and block/vdi code handling UUIDs. Mostly I fix
these by copying the unaligned field to/from a local variable.
In the case of qemu_uuid_bswap() I opted to change the API to
take and return the QemuUUID rather than taking a pointer to it,
which makes almost all the callsites simpler. This does mean
a struct copy but the struct is only 16 bytes and I didn't
judge any of the callsites performance-sensitive enough to care
about a struct copy of that size.

As usual, tested with "make check" only.

thanks
-- PMM


Peter Maydell (3):
  block/vpc: Don't take address of fields in packed structs
  block/vdi: Don't take address of fields in packed structs
  uuid: Make qemu_uuid_bswap() take and return a QemuUUID

 include/qemu/uuid.h  |  2 +-
 block/vdi.c          | 54 +++++++++++++++++++++++++++-----------------
 block/vpc.c          |  4 +++-
 hw/acpi/vmgenid.c    |  6 ++---
 tests/vmgenid-test.c |  2 +-
 util/uuid.c          | 10 ++++----
 6 files changed, 45 insertions(+), 33 deletions(-)

-- 
2.19.2


Re: [Qemu-devel] [PATCH 0/3] block: fix last address-of-packed-member warnings
Posted by Peter Maydell 5 years, 3 months ago
Ping?

thanks
-- PMM

On Mon, 10 Dec 2018 at 11:28, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This patchset fixes the remaining clang warnings in the block/ code
> about taking the address of a packed struct member, which are all
> in block/vpc and block/vdi code handling UUIDs. Mostly I fix
> these by copying the unaligned field to/from a local variable.
> In the case of qemu_uuid_bswap() I opted to change the API to
> take and return the QemuUUID rather than taking a pointer to it,
> which makes almost all the callsites simpler. This does mean
> a struct copy but the struct is only 16 bytes and I didn't
> judge any of the callsites performance-sensitive enough to care
> about a struct copy of that size.
>
> As usual, tested with "make check" only.
>
> thanks
> -- PMM
>
>
> Peter Maydell (3):
>   block/vpc: Don't take address of fields in packed structs
>   block/vdi: Don't take address of fields in packed structs
>   uuid: Make qemu_uuid_bswap() take and return a QemuUUID
>
>  include/qemu/uuid.h  |  2 +-
>  block/vdi.c          | 54 +++++++++++++++++++++++++++-----------------
>  block/vpc.c          |  4 +++-
>  hw/acpi/vmgenid.c    |  6 ++---
>  tests/vmgenid-test.c |  2 +-
>  util/uuid.c          | 10 ++++----
>  6 files changed, 45 insertions(+), 33 deletions(-)

Re: [Qemu-devel] [PATCH 0/3] block: fix last address-of-packed-member warnings
Posted by Kevin Wolf 5 years, 3 months ago
Am 10.12.2018 um 12:26 hat Peter Maydell geschrieben:
> This patchset fixes the remaining clang warnings in the block/ code
> about taking the address of a packed struct member, which are all
> in block/vpc and block/vdi code handling UUIDs. Mostly I fix
> these by copying the unaligned field to/from a local variable.
> In the case of qemu_uuid_bswap() I opted to change the API to
> take and return the QemuUUID rather than taking a pointer to it,
> which makes almost all the callsites simpler. This does mean
> a struct copy but the struct is only 16 bytes and I didn't
> judge any of the callsites performance-sensitive enough to care
> about a struct copy of that size.
> 
> As usual, tested with "make check" only.

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [PATCH 0/3] block: fix last address-of-packed-member warnings
Posted by Peter Maydell 5 years, 2 months ago
On Fri, 18 Jan 2019 at 16:17, Kevin Wolf <kwolf@redhat.com> wrote:
>
> Am 10.12.2018 um 12:26 hat Peter Maydell geschrieben:
> > This patchset fixes the remaining clang warnings in the block/ code
> > about taking the address of a packed struct member, which are all
> > in block/vpc and block/vdi code handling UUIDs. Mostly I fix
> > these by copying the unaligned field to/from a local variable.
> > In the case of qemu_uuid_bswap() I opted to change the API to
> > take and return the QemuUUID rather than taking a pointer to it,
> > which makes almost all the callsites simpler. This does mean
> > a struct copy but the struct is only 16 bytes and I didn't
> > judge any of the callsites performance-sensitive enough to care
> > about a struct copy of that size.
> >
> > As usual, tested with "make check" only.
>
> Thanks, applied to the block branch.

Ping? These don't seem to have made it into master yet.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/3] block: fix last address-of-packed-member warnings
Posted by Kevin Wolf 5 years, 2 months ago
Am 01.02.2019 um 11:30 hat Peter Maydell geschrieben:
> On Fri, 18 Jan 2019 at 16:17, Kevin Wolf <kwolf@redhat.com> wrote:
> >
> > Am 10.12.2018 um 12:26 hat Peter Maydell geschrieben:
> > > This patchset fixes the remaining clang warnings in the block/ code
> > > about taking the address of a packed struct member, which are all
> > > in block/vpc and block/vdi code handling UUIDs. Mostly I fix
> > > these by copying the unaligned field to/from a local variable.
> > > In the case of qemu_uuid_bswap() I opted to change the API to
> > > take and return the QemuUUID rather than taking a pointer to it,
> > > which makes almost all the callsites simpler. This does mean
> > > a struct copy but the struct is only 16 bytes and I didn't
> > > judge any of the callsites performance-sensitive enough to care
> > > about a struct copy of that size.
> > >
> > > As usual, tested with "make check" only.
> >
> > Thanks, applied to the block branch.
> 
> Ping? These don't seem to have made it into master yet.

It's in my queue.

Kevin