<arei.gong...@huawei.com> writes:
> From: Gonglei <arei.gong...@huawei.com>
>
> When scsi_bus_legacy_add_drive() return NULL, meanwhile err will
> be not NULL, which will casue memory leak and missing error message.
>
> Signed-off-by: Gonglei <arei.gong...@huawei.com>
> ---
> hw/usb/dev-storage.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> index 5c51ac0..cabd053 100644
> --- a/hw/usb/dev-storage.c
> +++ b/hw/usb/dev-storage.c
> @@ -599,7 +599,6 @@ static int usb_msd_initfn_storage(USBDevice *dev)
> {
> MSDState *s = DO_UPCAST(MSDState, dev, dev);
> BlockDriverState *bs = s->conf.bs;
> - SCSIDevice *scsi_dev;
> Error *err = NULL;
>
> if (!bs) {
> @@ -625,10 +624,12 @@ static int usb_msd_initfn_storage(USBDevice *dev)
> usb_desc_init(dev);
> scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(dev),
> &usb_msd_scsi_info_storage, NULL);
> - scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable,
> - s->conf.bootindex, dev->serial,
> - &err);
> - if (!scsi_dev) {
> + scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable,
> + s->conf.bootindex, dev->serial,
> + &err);
> + if (err) {
> + error_report("%s", error_get_pretty(err));
> + error_free(err);
> return -1;
> }
> s->bus.qbus.allow_hotplug = 0;
I'd keep the original condition and just fix the error message loss /
error object leak:
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index ae4efcb..f731b0a 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -624,6 +624,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
s->conf.bootindex, dev->serial,
&err);
if (!scsi_dev) {
+ error_report("%s", error_get_pretty(err));
+ error_free(err);
return -1;
}
s->bus.qbus.allow_hotplug = 0;
Partly because it makes the fix more obvious, partly because I prefer
checking the function value when it is available. Matter of taste.
scsi_hot_add() in pci-hotplug-old.c also loses the error message. Would
you care to fix that, too?
