[Qemu-devel] [PATCH 4/4] Added error checking for msix_init.

[Qemu-devel] [PATCH 4/4] Added error checking for msix_init.

[Shreya Shrivastava]

Add checks for negative return value to uses of msix_init.
Signed-off-by: Shreya Shrivastava <pink89@sigaint.org>

---
 hw/usb/hcd-xhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 4acf0c6..cb854aa 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3705,10 +3705,11 @@ static void usb_xhci_realize(struct PCIDevice
*dev, Error **errp)

     if (xhci->msix != ON_OFF_AUTO_OFF) {
         /* TODO check for errors */
-        msix_init(dev, xhci->numintrs,
+        ret = msix_init(dev, xhci->numintrs,
                   &xhci->mem, 0, OFF_MSIX_TABLE,
                   &xhci->mem, 0, OFF_MSIX_PBA,
                   0x90);
+        assert(ret >= 0);
     }
 }

-- 
1.9.1

Re: [Qemu-devel] [PATCH 4/4] Added error checking for msix_init.

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]

On Sun, Oct 16, 2016 at 12:53:16AM -0000, Shreya Shrivastava wrote:
> Add checks for negative return value to uses of msix_init.
> Signed-off-by: Shreya Shrivastava <pink89@sigaint.org>
> 
> ---
>  hw/usb/hcd-xhci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index 4acf0c6..cb854aa 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -3705,10 +3705,11 @@ static void usb_xhci_realize(struct PCIDevice
> *dev, Error **errp)
> 
>      if (xhci->msix != ON_OFF_AUTO_OFF) {
>          /* TODO check for errors */
> -        msix_init(dev, xhci->numintrs,
> +        ret = msix_init(dev, xhci->numintrs,
>                    &xhci->mem, 0, OFF_MSIX_TABLE,
>                    &xhci->mem, 0, OFF_MSIX_PBA,
>                    0x90);
> +        assert(ret >= 0);
>      }
>  }

Please handle the error instead of adding an assert:

1. Use error_setg_errno(errp, -ret, "MSI-X initialization failed") so
   the caller knows an error occurred.

2. Look at the resources initialized by this function and pick
   corresponding cleanup calls from usb_xhci_exit() so they are freed.
   For example xhci->mfwrap_timer was created so timer_free() needs to
   be called.

   (This is easier if you can move the msix_init() call further up
   before resources are initialized just like the msi_init() call.  But
   you need to check if msix_init() relies on anything before moving the
   call up.)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]