[libvirt] [ocaml PATCH] Cast virError* enums to int for comparisons with 0

Pino Toscano posted 1 patch 5 years, 4 months ago
Failed in applying to current master (apply log)
libvirt/libvirt_c_epilogue.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[libvirt] [ocaml PATCH] Cast virError* enums to int for comparisons with 0
Posted by Pino Toscano 5 years, 4 months ago
The actual type of an enum in C is implementation defined when there are
no negative values, and thus it can be int, or uint.  This is the case
of the virError* enums in libvirt, as they do not have negative values.

Hence, to avoid hitting tautological comparison errors when checking
their rage, temporarly cast the enum values to int when checking they
are not negative.  The check is there to ensure the value is within the
range of the OCaml type used to represent it.

Signed-off-by: Pino Toscano <ptoscano@redhat.com>
---
 libvirt/libvirt_c_epilogue.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libvirt/libvirt_c_epilogue.c b/libvirt/libvirt_c_epilogue.c
index 29656a4..4e75d2f 100644
--- a/libvirt/libvirt_c_epilogue.c
+++ b/libvirt/libvirt_c_epilogue.c
@@ -153,7 +153,7 @@ Val_err_number (virErrorNumber code)
   CAMLparam0 ();
   CAMLlocal1 (rv);
 
-  if (0 <= code && code <= MAX_VIR_CODE)
+  if (0 <= (int) code && code <= MAX_VIR_CODE)
     rv = Val_int (code);
   else {
     rv = caml_alloc (1, 0);	/* VIR_ERR_UNKNOWN (code) */
@@ -169,7 +169,7 @@ Val_err_domain (virErrorDomain code)
   CAMLparam0 ();
   CAMLlocal1 (rv);
 
-  if (0 <= code && code <= MAX_VIR_DOMAIN)
+  if (0 <= (int) code && code <= MAX_VIR_DOMAIN)
     rv = Val_int (code);
   else {
     rv = caml_alloc (1, 0);	/* VIR_FROM_UNKNOWN (code) */
@@ -185,7 +185,7 @@ Val_err_level (virErrorLevel code)
   CAMLparam0 ();
   CAMLlocal1 (rv);
 
-  if (0 <= code && code <= MAX_VIR_LEVEL)
+  if (0 <= (int) code && code <= MAX_VIR_LEVEL)
     rv = Val_int (code);
   else {
     rv = caml_alloc (1, 0);	/* VIR_ERR_UNKNOWN_LEVEL (code) */
-- 
2.17.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ocaml PATCH] Cast virError* enums to int for comparisons with 0
Posted by Andrea Bolognani 5 years, 4 months ago
On Fri, 2018-11-02 at 15:52 +0100, Pino Toscano wrote:
> The actual type of an enum in C is implementation defined when there are
> no negative values, and thus it can be int, or uint.  This is the case
> of the virError* enums in libvirt, as they do not have negative values.
> 
> Hence, to avoid hitting tautological comparison errors when checking
> their rage, temporarly cast the enum values to int when checking they
> are not negative.  The check is there to ensure the value is within the
> range of the OCaml type used to represent it.
> 
> Signed-off-by: Pino Toscano <ptoscano@redhat.com>
> ---
>  libvirt/libvirt_c_epilogue.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Looks sane to me.

If that's enough for you, then you can have my

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

and push. I just really want

  https://ci.centos.org/view/libvirt/job/libvirt-ocaml-build/

to turn green :)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ocaml PATCH] Cast virError* enums to int for comparisons with 0
Posted by Richard W.M. Jones 5 years, 4 months ago
On Fri, Nov 02, 2018 at 03:52:23PM +0100, Pino Toscano wrote:
> The actual type of an enum in C is implementation defined when there are
> no negative values, and thus it can be int, or uint.  This is the case
> of the virError* enums in libvirt, as they do not have negative values.
> 
> Hence, to avoid hitting tautological comparison errors when checking
> their rage, temporarly cast the enum values to int when checking they

s/rage/range/

The patch itself is fine:

ACK.

Rich.

> are not negative.  The check is there to ensure the value is within the
> range of the OCaml type used to represent it.
> 
> Signed-off-by: Pino Toscano <ptoscano@redhat.com>
> ---
>  libvirt/libvirt_c_epilogue.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libvirt/libvirt_c_epilogue.c b/libvirt/libvirt_c_epilogue.c
> index 29656a4..4e75d2f 100644
> --- a/libvirt/libvirt_c_epilogue.c
> +++ b/libvirt/libvirt_c_epilogue.c
> @@ -153,7 +153,7 @@ Val_err_number (virErrorNumber code)
>    CAMLparam0 ();
>    CAMLlocal1 (rv);
>  
> -  if (0 <= code && code <= MAX_VIR_CODE)
> +  if (0 <= (int) code && code <= MAX_VIR_CODE)
>      rv = Val_int (code);
>    else {
>      rv = caml_alloc (1, 0);	/* VIR_ERR_UNKNOWN (code) */
> @@ -169,7 +169,7 @@ Val_err_domain (virErrorDomain code)
>    CAMLparam0 ();
>    CAMLlocal1 (rv);
>  
> -  if (0 <= code && code <= MAX_VIR_DOMAIN)
> +  if (0 <= (int) code && code <= MAX_VIR_DOMAIN)
>      rv = Val_int (code);
>    else {
>      rv = caml_alloc (1, 0);	/* VIR_FROM_UNKNOWN (code) */
> @@ -185,7 +185,7 @@ Val_err_level (virErrorLevel code)
>    CAMLparam0 ();
>    CAMLlocal1 (rv);
>  
> -  if (0 <= code && code <= MAX_VIR_LEVEL)
> +  if (0 <= (int) code && code <= MAX_VIR_LEVEL)
>      rv = Val_int (code);
>    else {
>      rv = caml_alloc (1, 0);	/* VIR_ERR_UNKNOWN_LEVEL (code) */
> -- 
> 2.17.2
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list