[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] slirp: Don't mark struct ipq or struct ipas
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] slirp: Don't mark struct ipq or struct ipasfrag as packed |
Date: |
Tue, 22 Jan 2019 18:57:26 +0000 |
On Tue, 22 Jan 2019 at 18:55, Daniel P. Berrangé <address@hidden> wrote:
>
> On Tue, Jan 22, 2019 at 06:18:22PM +0000, Peter Maydell wrote:
> > There is no reason to mark the struct ipq and struct ipasfrag as
> > packed: they are naturally aligned anyway, and are not representing
> > any on-the-wire packet format. Indeed they vary in size depending on
> > the size of pointers on the host system, because the 'struct qlink'
> > members include 'void *' fields.
> >
> > Dropping the 'packed' annotation fixes clang -Waddress-of-packed-member
> > warnings and probably lets the compiler generate better code too.
> >
> > The only thing we do care about in the layout of the struct is
> > that the frag_link matches up with the ipf_link of the struct
> > ipasfrag, as documented in the comment on that struct; assert
> > at build time that this is the case.
> >
> > Signed-off-by: Peter Maydell <address@hidden>
> > ---
> > slirp/ip.h | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/slirp/ip.h b/slirp/ip.h
> > index 243b6c8b249..20614f3b53e 100644
> > --- a/slirp/ip.h
> > +++ b/slirp/ip.h
>
> Just here there's a misleading comment
>
> * size 28 bytes
>
> that should be purged
I noticed that one but left it alone on the basis that I
wasn't sure what to replace it with, and as it is it
provides the useful information of "somebody at some point
thought the size of this struct mattered but they were
badly confused, so tread with caution" :-)
thanks
-- PMM