Hi On Tue, May 5, 2026 at 1:57 PM Peter Maydell <[email protected]> wrote: > > On Mon, 4 May 2026 at 12:36, Marc-André Lureau > <[email protected]> wrote: > > > > The type-checking macros (IPMI_BMC(val)) which crash on NULL. > > > > Signed-off-by: Marc-André Lureau <[email protected]> > > --- > > hw/ipmi/ipmi.c | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c > > index b49affaccec..d532d38e556 100644 > > --- a/hw/ipmi/ipmi.c > > +++ b/hw/ipmi/ipmi.c > > @@ -95,7 +95,14 @@ static const TypeInfo ipmi_interface_type_info = { > > static void isa_ipmi_bmc_check(const Object *obj, const char *name, > > Object *val, Error **errp) > > { > > - IPMIBmc *bmc = IPMI_BMC(val); > > + IPMIBmc *bmc; > > + > > + if (!val) { > > + error_setg(errp, "%s cannot be set to NULL", name); > > + return; > > + } > > + > > + bmc = IPMI_BMC(val); > > > > if (bmc->intf) > > error_setg(errp, "BMC object is already in use"); > > I was looking through the code to check where the type macros fall over > on NULL pointers, and I couldn't find it, so I did this little test, > and as far as I can tell they don't assert, they just return NULL: > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 2ecad3c503..32569c1f09 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -66,6 +66,7 @@ > #include "e820_memory_layout.h" > #include "trace.h" > #include "sev.h" > +#include "hw/ipmi/ipmi.h" > #include CONFIG_DEVICES > > #ifdef CONFIG_XEN_EMU > @@ -1580,6 +1581,9 @@ static void pc_machine_initfn(Object *obj) > PCMachineState *pcms = PC_MACHINE(obj); > PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms); > > + void *foo = IPMI_BMC(NULL); > + printf("foo = %p\n", foo); > + > #ifdef CONFIG_VMPORT > pcms->vmport = ON_OFF_AUTO_AUTO; > #else > > This code runs on the pc machine startup and prints "foo = (nil)" > without problems, even on a debug build (which enables > CONFIG_QOM_CAST_DEBUG). > > Is the problem you saw really with the cast macro, and not with > this function's explicit dereference of bmc in "bmc->intf" ?
You're right; it's with the "bmc->intf", I'll update the commit message.
