Re: [Qemu-devel] [qemu-s390x] [PATCH 2/4] pc-bios/s390-ccw: fix loadparm initialization and int conversion

[Collin Walling]

On 04/16/2018 08:27 AM, Thomas Huth wrote:
> On 14.04.2018 00:08, Collin Walling wrote:
>> Rename the loadparm char array in main.c to loadparm_str and
>> increased the size by one byte to account for a null termination
>> when converting the loadparm string to an int  via atoui. We
>> also allow the boot menu to be enabled when loadparm is set to
>> an empty string or a series of spaces.
>>
>> Signed-off-by: Collin Walling <address@hidden>
>> Reported-by: Vasily Gorbik <address@hidden>
>> Reviewed-by: Thomas Huth <address@hidden>
>> ---
>>  hw/s390x/ipl.c          |  2 ++
>>  pc-bios/s390-ccw/main.c | 14 +++++++-------
>>  2 files changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index fdeaec3..23b5b54 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -352,6 +352,8 @@ int s390_ipl_set_loadparm(uint8_t *loadparm)
>>              loadparm[i] = ascii2ebcdic[(uint8_t) lp[i]];
>>          }
>>  
>> +        memset(loadparm + i, 0x40, 8 - i); /* fill with EBCDIC spaces */
>> +
>>          g_free(lp);
>>          return 0;
>>      }
> 
> When compiling this code, my GCC (v4.8.5) complains:
> 
>   CC      s390x-softmmu/hw/s390x/ipl.o
> In file included from /usr/include/string.h:638:0,
>                  from /home/thuth/devel/qemu/include/qemu/osdep.h:69,
>                  from /home/thuth/devel/qemu/hw/s390x/ipl.c:14:
> In function ‘memset’,
>     inlined from ‘s390_ipl_set_loadparm’ at
> /home/thuth/devel/qemu/hw/s390x/ipl.c:376:15:
> /usr/include/bits/string3.h:81:30: error: call to
> ‘__warn_memset_zero_len’ declared with attribute warning: memset used
> with constant zero length parameter; this could be due to transposed
> parameters [-Werror]
>        __warn_memset_zero_len ();
> 
> I guess this might happen due to some internal loop unrolling of GCC or
> something similar ... to make sure that we can compile the code also
> without warnings, could you please add a check around the memset à la:
> 
>     if (i < 8) {
>         memset(loadparm + i, 0x40, 8 - i); /* fill with EBCDIC spaces */
>     }
> 
>  Thanks,
>   Thomas
> 

Can do.

-- 
Respectfully,
- Collin Walling