These patches fix the clang -Waddress-of-packed-member warnings
in the slirp code. For slirp, the structs marked as packed
are already naturally packed/aligned, so adding the 'packed'
attribute doesn't change their layout but means the compiler
has to assume they might be misaligned. Instead, for the structs
which map to on-the-wire packet formats, we add build time
assertions that they have the sizes we think they should.

The 'struct ipq' and 'struct ipasfrag' aren't on-the-wire
packet representations, so it's not clear why they were
marked packed in the first place. (They contain host pointers
and so have a size that depends on the host.) Here we just
assert the only thing that's documented as a layout restriction:
that struct ipq's frag_link and struct ipasfrag's ipf_link
are at the same offset.

Together, this fixes all the -Waddress-of-packed-member warnings
in slirp, which I think was the only set remaining without
patches at least being on-list. (Some of the block/ warning fixes
are still working their way through the relevant trees.)

thanks
-- PMM

Peter Maydell (2):
  slirp: Avoid marking naturally packed structs as QEMU_PACKED
  slirp: Don't mark struct ipq or struct ipasfrag as packed

 slirp/ip.h       |  7 +++++--
 slirp/ip6.h      | 12 ++++++++++--
 slirp/ip6_icmp.h | 20 +++++++++++++++-----
 3 files changed, 30 insertions(+), 9 deletions(-)

-- 
2.20.1

On 1/22/19 12:18 PM, Peter Maydell wrote:
> These patches fix the clang -Waddress-of-packed-member warnings
> in the slirp code. For slirp, the structs marked as packed
> are already naturally packed/aligned, so adding the 'packed'
> attribute doesn't change their layout but means the compiler
> has to assume they might be misaligned. Instead, for the structs
> which map to on-the-wire packet formats, we add build time
> assertions that they have the sizes we think they should.
> 
> The 'struct ipq' and 'struct ipasfrag' aren't on-the-wire
> packet representations, so it's not clear why they were
> marked packed in the first place. (They contain host pointers
> and so have a size that depends on the host.) Here we just
> assert the only thing that's documented as a layout restriction:
> that struct ipq's frag_link and struct ipasfrag's ipf_link
> are at the same offset.
> 
> Together, this fixes all the -Waddress-of-packed-member warnings
> in slirp, which I think was the only set remaining without
> patches at least being on-list. (Some of the block/ warning fixes
> are still working their way through the relevant trees.)

How does this series play with the ongoing effort to make libslirp an
independent project?  I'm assuming your use of QEMU_BUILD_BUG_ON() will
have to be translated into whatever libslirp-specific mechanism we want
to split off into that project.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Hello,

Peter Maydell, le mar. 22 janv. 2019 18:18:20 +0000, a ecrit:
> These patches fix the clang -Waddress-of-packed-member warnings
> in the slirp code.

Applied to my tree, thanks!

Eric Blake, le mar. 22 janv. 2019 13:27:32 -0600, a ecrit:
> How does this series play with the ongoing effort to make libslirp an
> independent project?  I'm assuming your use of QEMU_BUILD_BUG_ON() will
> have to be translated into whatever libslirp-specific mechanism we want
> to split off into that project.

Indeed, but I guess libslirp will want to have such kind of macro
anyway, so I don't see a reason to hold this change.

Samuel