Re: [PATCH 1/1] s390x/s390-virtio-ccw: fix loadparm property getter

[Cornelia Huck]

On Thu, 23 Jul 2020 18:37:50 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 23/07/2020 18.27, Halil Pasic wrote:
> > The function machine_get_loadparm() is supposed to produce as C-string,  
> 
> sed "s/ as / a /"
> 
> > that is a null-terminated one, but it does not. ElectricFence can detect  
> 
> maybe: sed "s/null/NUL/"

"NUL" seems to be the more commonly used form in QEMU, so I went ahead
and changed it.
> 
> > this problem if the loadparm machine property is used.
> > 
> > Let us make the returned string a null-terminated one.  
> 
> dito
> 
> > Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> > Fixes: 7104bae9de ("hw/s390x: provide loadparm property for the machine")
> > ---
> >  hw/s390x/s390-virtio-ccw.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> > index 8cc2f25d8a..e0e4a69ac8 100644
> > --- a/hw/s390x/s390-virtio-ccw.c
> > +++ b/hw/s390x/s390-virtio-ccw.c
> > @@ -701,8 +701,12 @@ bool hpage_1m_allowed(void)
> >  static char *machine_get_loadparm(Object *obj, Error **errp)
> >  {
> >      S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
> > +    char *loadparm_str;
> >  
> > -    return g_memdup(ms->loadparm, sizeof(ms->loadparm));
> > +    /* make a null-terminated string */  
> 
> maybe: sed "s/null/NUL/"
> 
> > +     loadparm_str = g_memdup(ms->loadparm, sizeof(ms->loadparm) + 1);  
> 
> Wrong indentation.
> 
> > +    loadparm_str[sizeof(ms->loadparm)] = 0;
> > +    return loadparm_str;
> >  }  
> 
> With the cosmetics fixed:
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thanks, queued to s390-fixes (with the nits fixed.)