Re: [PATCH 1/2] hw/s390x: Declare loadparm as char[LOADPARM_LEN]

[Thomas Huth]

On 30/12/2024 18.02, Philippe Mathieu-Daudé wrote:
> ping for this single cleanup patch?
>
> On 20/11/24 09:52, Philippe Mathieu-Daudé wrote:
> > "hw/s390x/ipl/qipl.h" defines loadparm[] length as LOADPARM_LEN,
> > use that instead of the magic '8' value. Use a char type for
> > char buffer.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   hw/s390x/ccw-device.h               | 5 +++--
> >   hw/s390x/ipl.h                      | 2 +-
> >   include/hw/qdev-properties-system.h | 2 +-
> >   include/hw/s390x/s390-virtio-ccw.h  | 3 ++-
> >   hw/core/qdev-properties-system.c    | 8 +++++---
> >   hw/s390x/ipl.c                      | 6 +++---
> >   hw/scsi/scsi-disk.c                 | 2 +-
> >   7 files changed, 16 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/s390x/ccw-device.h b/hw/s390x/ccw-device.h
> > index 4439feb140..94a9b35714 100644
> > --- a/hw/s390x/ccw-device.h
> > +++ b/hw/s390x/ccw-device.h
> > @@ -15,6 +15,7 @@
> >   #include "hw/qdev-core.h"
> >   #include "hw/s390x/css.h"
> >   #include "hw/s390x/css-bridge.h"
> > +#include "hw/s390x/ipl/qipl.h"
> >   struct CcwDevice {
> >       DeviceState parent_obj;
> > @@ -27,7 +28,7 @@ struct CcwDevice {
> >       /* The actual busid of the virtual subchannel. */
> >       CssDevId subch_id;
> >       /* If set, use this loadparm value when device is boot target */
> > -    uint8_t loadparm[8];
> > +    char loadparm[LOADPARM_LEN];

I'm not sure whether changing the type is such a good idea ... some spots 
that handle loadparm include EBCDIC encoding, and the unsigned type might 
have been used here for a reason? I'd better keep uint8_t unless you've 
double-checked all combinations...

 Thomas