Re: [Qemu-devel] [PATCH 11/14] hw/vfio/ccw: avoid taking address members in packed structs

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Apr 02, 2019 at 06:00:33PM +0200, Cornelia Huck wrote:
> On Fri, 29 Mar 2019 11:11:01 +0000
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> 
> > The GCC 9 compiler complains about many places in s390 code
> > that take the address of members of the 'struct SCHIB' which
> > is marked packed:
> > 
> > hw/vfio/ccw.c: In function ‘vfio_ccw_io_notifier_handler’:
> > hw/vfio/ccw.c:133:15: warning: taking address of packed member of ‘struct SCHIB’ may result in an unaligned pointer value \
> > [-Waddress-of-packed-member]
> >   133 |     SCSW *s = &sch->curr_status.scsw;
> >       |               ^~~~~~~~~~~~~~~~~~~~~~
> > hw/vfio/ccw.c:134:15: warning: taking address of packed member of ‘struct SCHIB’ may result in an unaligned pointer value \
> > [-Waddress-of-packed-member]
> >   134 |     PMCW *p = &sch->curr_status.pmcw;
> >       |               ^~~~~~~~~~~~~~~~~~~~~~
> > 
> > ...snip many more...
> > 
> > Almost all of these are just done for convenience to avoid
> > typing out long variable/field names when referencing struct
> > members. We can get most of this convenience by taking the
> > address of the 'struct SCHIB' instead, avoiding triggering
> > the compiler warnings.
> > 
> > In a couple of places we copy via a local variable which is
> > a technique already applied elsewhere in s390 code for this
> > problem.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  hw/vfio/ccw.c | 42 ++++++++++++++++++++++--------------------
> >  1 file changed, 22 insertions(+), 20 deletions(-)
> 
> I'm currently in the process of queuing this and the other three s390x
> fixes, but I'm inclined to do so for 4.1 (it feels a bit late in the
> cycle for 4.0.)
> 
> Other opinions?

It would be nice to be warning free for 4.0, but I agree that it feels
kind of late to be making these changes. They're not fixing real world
bugs, and even if you queue the s390 bits we're unlikely to get all the
others merged, especially the usb-mtp one is a nasty mess. So we'll
not be 100% warning free.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|