[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [qemu-s390x] [PATCH 11/14] hw/vfio/ccw: avoid taking address members
From: |
Thomas Huth |
Subject: |
Re: [qemu-s390x] [PATCH 11/14] hw/vfio/ccw: avoid taking address members in packed structs |
Date: |
Tue, 2 Apr 2019 18:05:15 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.0 |
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.
Thomas