Re: [PATCH 02/11] hw/i386/pc: Use qdev_prop_set_array()

[Kevin Wolf <kwolf@redhat.com>]

Am 11.09.2023 um 17:42 hat Peter Maydell geschrieben:
> On Fri, 8 Sept 2023 at 15:37, Kevin Wolf <kwolf@redhat.com> wrote:
> >
> > Instead of manually setting "foo-len" and "foo[i]" properties, build a
> > QList and use the new qdev_prop_set_array() helper to set the whole
> > array property with a single call.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  hw/i386/pc.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 54838c0c41..0e84e454cb 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -81,6 +81,7 @@
> >  #include "qapi/error.h"
> >  #include "qapi/qapi-visit-common.h"
> >  #include "qapi/qapi-visit-machine.h"
> > +#include "qapi/qmp/qlist.h"
> >  #include "qapi/visitor.h"
> >  #include "hw/core/cpu.h"
> >  #include "hw/usb.h"
> > @@ -1508,9 +1509,10 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> >          char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
> >                                                VIRTIO_IOMMU_RESV_MEM_T_MSI);
> >
> > -        object_property_set_uint(OBJECT(dev), "len-reserved-regions", 1, errp);
> > -        object_property_set_str(OBJECT(dev), "reserved-regions[0]",
> > -                                resv_prop_str, errp);
> > +        QList *reserved_regions = qlist_new();
> > +        qlist_append_str(reserved_regions, resv_prop_str);
> > +        qdev_prop_set_array(dev, "reserved-regions", reserved_regions);
> > +
> 
> The variable declaration should be at the top of the block;

It is at the top of the block, the only thing before it is another
variable declaration. Would you prefer to have the empty line removed or
after the declaration to make this visually clearer?

Kevin