[SLOF] [PATCH slof] tcgbios: Fix warnings

[Stefan Berger]

On 7/1/21 2:35 AM, Thomas Huth wrote:
> On 30/06/2021 05.16, Alexey Kardashevskiy wrote:
>> This fixes gcc warnings from -Waddress-of-packed-member and 
>> -Wzero-length-bounds.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik at ozlabs.ru>
>>
>> ---
>>
>> tpm_drivers.c: In function ‘spapr_send_crq_and_wait’:
>> tpm_drivers.c:153:2: warning: converting a packed ‘struct crq’ 
>> pointer (alignment 1) to a ‘uint64_t’ {aka ‘long long unsigned int’} 
>> pointer alignment 8) may result in an unaligned pointer value 
>> [-Waddress-of-packed-member]
>>    153 |  rc = hv_send_crq(unit, (uint64_t *)crq);
>>        |  ^~
>> tpm_drivers.c:34:8: note: defined here
>>     34 | struct crq {
>>        |        ^~~
>> tpm_drivers.c: In function ‘spapr_vtpm_senddata’:
>> tpm_drivers.c:346:2: warning: converting a packed ‘struct crq’ 
>> pointer (alignment 1) to a ‘uint64_t’ {aka ‘long long unsigned int’} 
>> pointer (alignment 8) may result in an unaligned pointer value 
>> [-Waddress-of-packed-member]
>>    346 |  rc = hv_send_crq(spapr_vtpm.unit, (uint64_t *)&crq);
>>        |  ^~
>> tpm_drivers.c:34:8: note: defined here
>>     34 | struct crq {
>>        |        ^~~
>>          [CC]    common-libs
>>          [CC]    common-libs
>> tcgbios.c: In function ‘tpm20_write_EfiSpecIdEventStruct’:
>> tcgbios.c:708:24: warning: array subscript ‘numAlgs’ is outside the 
>> bounds of an interior zero-length array ‘struct 
>> TCG_EfiSpecIdEventAlgorithmSize[0]’ [-Wzero-length-bounds]
>>    708 |   event.hdr.digestSizes[numAlgs].algorithmId =
>>        |   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~
>> In file included from tpm_drivers.h:20,
>>                   from tcgbios.c:27:
>> tcgbios_int.h:92:4: note: while referencing ‘digestSizes’
>>     92 |  } digestSizes[0];
>>        |    ^~~~~~~~~~~
>> tcgbios.c:710:24: warning: array subscript ‘numAlgs’ is outside the 
>> bounds of an interior zero-length array ‘struct 
>> TCG_EfiSpecIdEventAlgorithmSize[0]’ [-Wzero-length-bounds]
>>    710 |   event.hdr.digestSizes[numAlgs].digestSize = 
>> cpu_to_log16(hsize);
>>        |   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~
>> In file included from tpm_drivers.h:20,
>>                   from tcgbios.c:27:
>> tcgbios_int.h:92:4: note: while referencing ‘digestSizes’
>>     92 |  } digestSizes[0];
>>        |    ^~~~~~~~~~~
>> ---
>>   lib/libtpm/tcgbios_int.h | 2 +-
>>   lib/libtpm/tpm_drivers.c | 4 ++--
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/libtpm/tcgbios_int.h b/lib/libtpm/tcgbios_int.h
>> index 22df31dd504f..cc38455850f2 100644
>> --- a/lib/libtpm/tcgbios_int.h
>> +++ b/lib/libtpm/tcgbios_int.h
>> @@ -89,7 +89,7 @@ struct TCG_EfiSpecIdEventStruct {
>>       struct TCG_EfiSpecIdEventAlgorithmSize {
>>           uint16_t algorithmId;
>>           uint16_t digestSize;
>> -    } digestSizes[0];
>> +    } digestSizes[];
>>       /*
>>       uint8_t vendorInfoSize;
>>       uint8_t vendorInfo[0];
>> diff --git a/lib/libtpm/tpm_drivers.c b/lib/libtpm/tpm_drivers.c
>> index 85cb3098712d..4a4fde89ade8 100644
>> --- a/lib/libtpm/tpm_drivers.c
>> +++ b/lib/libtpm/tpm_drivers.c
>> @@ -150,7 +150,7 @@ static bool spapr_send_crq_and_wait(unsigned long 
>> unit,
>>         vtpm_drv_state_set(state1, VTPM_DRV_ERROR_NO_FAILURE);
>>   -    rc = hv_send_crq(unit, (uint64_t *)crq);
>> +    rc = hv_send_crq(unit, (uint64_t *)&crq->valid);
>>       if (rc != H_SUCCESS) {
>>           vtpm_drv_state_set(VTPM_DRV_STATE_WAIT_INIT,
>>                      VTPM_DRV_ERROR_TPM_CRQ_ERROR);
>> @@ -343,7 +343,7 @@ static bool spapr_vtpm_senddata(const uint8_t 
>> *const data, uint32_t len)
>>       vtpm_drv_state_set(VTPM_DRV_STATE_SEND_TPM_CMD,
>>                  VTPM_DRV_ERROR_NO_FAILURE);
>>   -    rc = hv_send_crq(spapr_vtpm.unit, (uint64_t *)&crq);
>> +    rc = hv_send_crq(spapr_vtpm.unit, (uint64_t *)&crq.valid);
>>         if (rc == H_SUCCESS)
>>           vtpm_drv_state_set(VTPM_DRV_STATE_WAIT_TPM_RSP,
>>
>
> All members of struct crq seem to be naturally aligned, so you could 
> also simply remove the "packed" attribute there.
> Anyway, taking the detour via the uint8_t valid field seems ok to me, 
> too, so:
>
> Reviewed-by: Thomas Huth <thuth at redhat.com>


Tested-by: Stefan Berger <stefanb at linux.ibm.com>