Re: [PATCH for-6.2 v6 3/7] spapr_drc.c: do not error_report() when drc->dev->id == NULL
On Sat, Aug 07, 2021 at 03:41:52PM +0200, Markus Armbruster wrote: > Daniel Henrique Barboza 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 > > Signed-off-by: Daniel Henrique Barboza > > --- > > 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 Folded into my tree. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [PATCH for-6.2 v6 3/7] spapr_drc.c: do not error_report() when drc->dev->id == NULL
Daniel Henrique Barboza 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 > Signed-off-by: Daniel Henrique Barboza > --- > 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
[PATCH for-6.2 v6 3/7] spapr_drc.c: do not error_report() when drc->dev->id == NULL
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 Signed-off-by: Daniel Henrique Barboza --- 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 -- 2.31.1