Message ID | 20210719200827.1507276-4-danielhb413@gmail.com |
---|---|
State | New |
Headers | show |
Series | DEVICE_UNPLUG_ERROR QAPI event | expand |
Daniel Henrique Barboza <danielhb413@gmail.com> writes: > The error_report() call in drc_unisolate_logical() is not considering > that drc->dev->id can be NULL, and the underlying functions error_report() > calls to do its job (vprintf(), g_strdup_printf() ...) has undefined > behavior when trying to handle "%s" with NULL arguments. > > Besides, there is no utility into reporting that an unknown device was > rejected by the guest. > > Reviewed-by: Greg Kurz <groug@kaod.org> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- > hw/ppc/spapr_drc.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c > index a2f2634601..a4d9496f76 100644 > --- a/hw/ppc/spapr_drc.c > +++ b/hw/ppc/spapr_drc.c > @@ -167,8 +167,11 @@ static uint32_t drc_unisolate_logical(SpaprDrc *drc) > } > > drc->unplug_requested = false; > - error_report("Device hotunplug rejected by the guest " > - "for device %s", drc->dev->id); > + > + if (drc->dev->id) { > + error_report("Device hotunplug rejected by the guest " > + "for device %s", drc->dev->id); > + } > > /* > * TODO: send a QAPI DEVICE_UNPLUG_ERROR event when This differs from PATCH 1 and 2 in that it actually fixes a crash bug. The alternative is something like error_report("Device hotunplug rejected by the guest " "for device %s", drc->dev->id ?: ""); If the maintainer is okay with dropping the message when the device has no ID, then so am I: Reviewed-by: Markus Armbruster <armbru@redhat.com>
On Sat, Aug 07, 2021 at 03:41:52PM +0200, Markus Armbruster wrote: > Daniel Henrique Barboza <danielhb413@gmail.com> writes: > > > The error_report() call in drc_unisolate_logical() is not considering > > that drc->dev->id can be NULL, and the underlying functions error_report() > > calls to do its job (vprintf(), g_strdup_printf() ...) has undefined > > behavior when trying to handle "%s" with NULL arguments. > > > > Besides, there is no utility into reporting that an unknown device was > > rejected by the guest. > > > > Reviewed-by: Greg Kurz <groug@kaod.org> > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > > --- > > hw/ppc/spapr_drc.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c > > index a2f2634601..a4d9496f76 100644 > > --- a/hw/ppc/spapr_drc.c > > +++ b/hw/ppc/spapr_drc.c > > @@ -167,8 +167,11 @@ static uint32_t drc_unisolate_logical(SpaprDrc *drc) > > } > > > > drc->unplug_requested = false; > > - error_report("Device hotunplug rejected by the guest " > > - "for device %s", drc->dev->id); > > + > > + if (drc->dev->id) { > > + error_report("Device hotunplug rejected by the guest " > > + "for device %s", drc->dev->id); > > + } > > > > /* > > * TODO: send a QAPI DEVICE_UNPLUG_ERROR event when > > This differs from PATCH 1 and 2 in that it actually fixes a crash bug. > > The alternative is something like > > error_report("Device hotunplug rejected by the guest " > "for device %s", drc->dev->id ?: ""); > > If the maintainer is okay with dropping the message when the device has > no ID, then so am I: I am, given how briefly this message has even existed - and through all that time it would have crashed if you tried. > Reviewed-by: Markus Armbruster <armbru@redhat.com> Folded into my tree.
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index a2f2634601..a4d9496f76 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -167,8 +167,11 @@ static uint32_t drc_unisolate_logical(SpaprDrc *drc) } drc->unplug_requested = false; - error_report("Device hotunplug rejected by the guest " - "for device %s", drc->dev->id); + + if (drc->dev->id) { + error_report("Device hotunplug rejected by the guest " + "for device %s", drc->dev->id); + } /* * TODO: send a QAPI DEVICE_UNPLUG_ERROR event when