Re: [PATCH v5 01/10] hw/fsi: Introduce IBM's Local bus

[Ninad Palsule]

Hello Daniel,

On 10/19/23 03:14, Daniel P. Berrangé wrote:
> On Wed, Oct 11, 2023 at 10:13:30AM -0500, Ninad Palsule wrote:
> > This is a part of patchset where IBM's Flexible Service Interface is
> > introduced.
> >
> > The LBUS is modelled to maintain the qdev bus hierarchy and to take
> > advantage of the object model to automatically generate the CFAM
> > configuration block. The configuration block presents engines in the
> > order they are attached to the CFAM's LBUS. Engine implementations
> > should subclass the LBusDevice and set the 'config' member of
> > LBusDeviceClass to match the engine's type.
> >
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
> > ---
> > v2:
> > - Incorporated Joel's review comments.
> > v5:
> > - Incorporated review comments by Cedric.
> > ---
> >   include/hw/fsi/lbus.h | 51 +++++++++++++++++++++++++
> >   include/qemu/bitops.h |  6 +++
> >   hw/fsi/lbus.c         | 87 +++++++++++++++++++++++++++++++++++++++++++
> >   hw/Kconfig            |  1 +
> >   hw/fsi/Kconfig        |  2 +
> >   hw/fsi/meson.build    |  1 +
> >   hw/meson.build        |  1 +
> >   7 files changed, 149 insertions(+)
> >   create mode 100644 include/hw/fsi/lbus.h
> >   create mode 100644 hw/fsi/lbus.c
> >   create mode 100644 hw/fsi/Kconfig
> >   create mode 100644 hw/fsi/meson.build
> > +DeviceState *lbus_create_device(FSILBus *bus, const char *type, uint32_t addr)
> > +{
> > +    DeviceState *dev;
> > +    FSILBusNode *node;
> > +    BusState *state = BUS(bus);
> > +
> > +    dev = qdev_new(type);
> > +    qdev_prop_set_uint8(dev, "address", addr);
> > +    qdev_realize_and_unref(dev, state, &error_fatal);
> > +
> > +    /* Move to post_load */
> > +    node = g_malloc(sizeof(struct FSILBusNode));
> This allocation pattern is discouraged in favour of:
>
>      node = g_new0(FSILBusNode, 1);

I am using g_malloc() because I want program to terminate. I don't think 
g_new0 provide this functionality. Please let me know.

Thanks for the review!

~Ninad

> > +    node->ldev = FSI_LBUS_DEVICE(dev);
> > +    QLIST_INSERT_HEAD(&bus->devices, node, next);
> > +
> > +    return dev;
> > +}
> With regards,
> Daniel