Some cleanup of libxl

[Xen-devel] [PATCH 11/15] libxl_usb: Fix wrong usage of asserts

Posted by Anthony PERARD 6 years, 7 months ago

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
index 3d389c0198..694870a3c3 100644
--- a/tools/libxl/libxl_usb.c
+++ b/tools/libxl/libxl_usb.c
@@ -148,7 +148,7 @@ static int libxl__device_from_usbctrl(libxl__gc *gc, uint32_t domid,
         break;
     default:
         assert(0); /* can't really happen. */
-        break;
+        return ERROR_INVAL;
     }
     device->devid           = usbctrl->devid;
     device->domid           = domid;
@@ -371,7 +371,7 @@ static int libxl__device_usbctrl_add_hvm(libxl__gc *gc, uint32_t domid,
         break;
     default:
         assert(0); /* Should not be possible. */
-        break;
+        return ERROR_INVAL;
     }
 
     flexarray_append_pair(qmp_args, "id",
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 11/15] libxl_usb: Fix wrong usage of asserts

Posted by Ian Jackson 6 years, 4 months ago

Anthony PERARD writes ("[PATCH 11/15] libxl_usb: Fix wrong usage of asserts"):
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

I'm not sure why you wouldn't just delete the breaks, rather than
replacing them "return" ?

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 11/15] libxl_usb: Fix wrong usage of asserts

Posted by Anthony PERARD 6 years, 4 months ago

On Tue, Sep 17, 2019 at 05:44:03PM +0100, Ian Jackson wrote:
> Anthony PERARD writes ("[PATCH 11/15] libxl_usb: Fix wrong usage of asserts"):
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> I'm not sure why you wouldn't just delete the breaks, rather than
> replacing them "return" ?

Because asserts aren't supposed to be compiled in a release build. I
know that's not the case in libxl...

I could replace the assert(0) by abort() instead, they won't be need for
a break or return after it.

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 11/15] libxl_usb: Fix wrong usage of asserts

Posted by Ian Jackson 6 years, 4 months ago

Anthony PERARD writes ("Re: [PATCH 11/15] libxl_usb: Fix wrong usage of asserts"):
> On Tue, Sep 17, 2019 at 05:44:03PM +0100, Ian Jackson wrote:
> > Anthony PERARD writes ("[PATCH 11/15] libxl_usb: Fix wrong usage of asserts"):
> > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> > 
> > I'm not sure why you wouldn't just delete the breaks, rather than
> > replacing them "return" ?
> 
> Because asserts aren't supposed to be compiled in a release build. I
> know that's not the case in libxl...

Oh I see.

> I could replace the assert(0) by abort() instead, they won't be need for
> a break or return after it.

Yes, please.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 11/15] libxl_usb: Fix wrong usage of asserts

Posted by Anthony PERARD 6 years, 4 months ago

Replace the assert(0) by abort() since the intention in libxl is that
asserts are always compiled in. This patch makes its clear and removes
the need to deal with asserts been compiled out.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    v2:
    - replace asserts by abort().

 tools/libxl/libxl_usb.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
index 3d389c019822..9f72857d87af 100644
--- a/tools/libxl/libxl_usb.c
+++ b/tools/libxl/libxl_usb.c
@@ -147,8 +147,7 @@ static int libxl__device_from_usbctrl(libxl__gc *gc, uint32_t domid,
         device->backend_kind = LIBXL__DEVICE_KIND_NONE;
         break;
     default:
-        assert(0); /* can't really happen. */
-        break;
+        abort(); /* can't really happen. */
     }
     device->devid           = usbctrl->devid;
     device->domid           = domid;
@@ -370,8 +369,7 @@ static int libxl__device_usbctrl_add_hvm(libxl__gc *gc, uint32_t domid,
         flexarray_append_pair(qmp_args, "p3", GCSPRINTF("%d", usbctrl->ports));
         break;
     default:
-        assert(0); /* Should not be possible. */
-        break;
+        abort(); /* Should not be possible. */
     }
 
     flexarray_append_pair(qmp_args, "id",
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 11/15] libxl_usb: Fix wrong usage of asserts

Posted by Ian Jackson 6 years, 4 months ago

Anthony PERARD writes ("[PATCH v2 11/15] libxl_usb: Fix wrong usage of asserts"):
> Replace the assert(0) by abort() since the intention in libxl is that
> asserts are always compiled in. This patch makes its clear and removes
> the need to deal with asserts been compiled out.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel