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

[Cornelia Huck]

On Tue, 2 Apr 2019 18:05:15 +0200
Thomas Huth <address@hidden> wrote:

> On 02/04/2019 18.00, Cornelia Huck wrote:
> > On Fri, 29 Mar 2019 11:11:01 +0000
> > Daniel P. Berrangé <address@hidden> 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é <address@hidden>
> >> ---
> >>  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?  
> 
> IMHO it would be nice to get rid of the compiler warnings for the
> release. Multiple people reviewed the patches, so I think it should
> still be fine to include them.

Still need to run my smoke tests, but I can send a pull req tomorrow
for -rc3.