Re: [PATCH v3 1/9] pc-bios: s390x: cio.c cleanup and compile fix

[Janosch Frank <frankja@linux.ibm.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2724 bytes --]

On 5/27/20 10:24 AM, Christian Borntraeger wrote:
> 
> 
> On 27.05.20 09:49, Janosch Frank wrote:
>> Let's initialize the structs at the beginning to ease reading and also
>> zeroing all other fields. This also makes the compiler stop
>> complaining about sense_id_ccw.flags being ored into when it's not
>> initialized.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
>> ---
>>  pc-bios/s390-ccw/cio.c | 40 ++++++++++++++++++++++------------------
>>  1 file changed, 22 insertions(+), 18 deletions(-)
>>
>> diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c
>> index 339ec5fbe7..dbae1e240e 100644
>> --- a/pc-bios/s390-ccw/cio.c
>> +++ b/pc-bios/s390-ccw/cio.c
>> @@ -49,13 +49,13 @@ void enable_subchannel(SubChannelId schid)
>>  
>>  uint16_t cu_type(SubChannelId schid)
>>  {
>> -    Ccw1 sense_id_ccw;
>>      SenseId sense_data;
>> -
>> -    sense_id_ccw.cmd_code = CCW_CMD_SENSE_ID;
>> -    sense_id_ccw.cda = ptr2u32(&sense_data);
>> -    sense_id_ccw.count = sizeof(sense_data);
>> -    sense_id_ccw.flags |= CCW_FLAG_SLI;
>> +    Ccw1 sense_id_ccw = {
>> +        .cmd_code = CCW_CMD_SENSE_ID,
>> +        .count = sizeof(sense_data),
>> +        .flags = CCW_FLAG_SLI,
>> +        .cda = ptr2u32(&sense_data),
>> +    };
> 
> nitpicking, since you change the order anyway you _could_ swap count and flag
> to match the order of struct Ccw. Only necessary when redoing.

Sure, if it makes you happy :-)

> 
> 
>>  
>>      if (do_cio(schid, CU_TYPE_UNKNOWN, ptr2u32(&sense_id_ccw), CCW_FMT1)) {
>>          panic("Failed to run SenseID CCw\n");
>> @@ -67,13 +67,13 @@ uint16_t cu_type(SubChannelId schid)
>>  int basic_sense(SubChannelId schid, uint16_t cutype, void *sense_data,
>>                   uint16_t data_size)
>>  {
>> -    Ccw1 senseCcw;
>> +    Ccw1 senseCcw = {
>> +        .cmd_code = CCW_CMD_BASIC_SENSE,
>> +        .count = data_size,
>> +        .cda = ptr2u32(sense_data),
>> +    };
>>      Irb irb;
>>  
>> -    senseCcw.cmd_code = CCW_CMD_BASIC_SENSE;
>> -    senseCcw.cda = ptr2u32(sense_data);
>> -    senseCcw.count = data_size;
>> -
> 
> here it is fine, due to the lack of flags. 
> Was this actually a bug before that senseCcw.flags was
> not zeroed out?

I'd guess so, seems like the compiler went into other code paths with my
recent changes and then started complaining.
But it makes sense to clean them up anyway.

> 
> 
> [...]
> 
> Other than that
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>

Thanks!



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]