[Spice-devel] [PATCH spice-common 3/3] SpiceROP: Do not compare to constant

[Frediano Ziglio]

> 
> Silence compile-time warning with clang:
>  comparison of constant 16 with expression of type 'SpiceROP' is always
>  true [-Wtautological-constant-out-of-range-compare]
> ---
>  common/pixman_utils.c | 4 ++--
>  common/pixman_utils.h | 3 ++-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/common/pixman_utils.c b/common/pixman_utils.c
> index 81806c9..5a03ff2 100644
> --- a/common/pixman_utils.c
> +++ b/common/pixman_utils.c
> @@ -311,7 +311,7 @@ void spice_pixman_fill_rect_rop(pixman_image_t *dest,
>      spice_assert(height > 0);
>      spice_assert(x + width <= pixman_image_get_width(dest));
>      spice_assert(y + height <= pixman_image_get_height(dest));
> -    spice_assert(rop < 16);
> +    spice_assert(rop < SPICE_ROP_END);
>  
>      if (depth == 8) {
>          solid_rop_8_func_t rop_func = solid_rops_8[rop];

What?
Honestly I think either we are enabling too much warning or clang is wrong!
This is C, not C++, enumerators can be different quite easily so there is no
reason to assume that (I would open a bug to clang if this is inside -Wall).
I would try something like

    spice_assert((unsigned) rop < 16);

> @@ -462,7 +462,7 @@ void spice_pixman_tile_rect_rop(pixman_image_t *dest,
>      spice_assert(height > 0);
>      spice_assert(x + width <= pixman_image_get_width(dest));
>      spice_assert(y + height <= pixman_image_get_height(dest));
> -    spice_assert(rop < 16);
> +    spice_assert(rop < SPICE_ROP_END);
>      spice_assert(depth == spice_pixman_image_get_bpp(tile));
>  
>      tile_start_x = (x - offset_x) % tile_width;
> diff --git a/common/pixman_utils.h b/common/pixman_utils.h
> index f892777..7335663 100644
> --- a/common/pixman_utils.h
> +++ b/common/pixman_utils.h
> @@ -61,7 +61,8 @@ typedef enum {
>      SPICE_ROP_COPY_INVERTED, /* 0xc    NOT src */
>      SPICE_ROP_OR_INVERTED,   /* 0xd    (NOT src) OR dst */
>      SPICE_ROP_NAND,          /* 0xe    (NOT src) OR (NOT dst) */
> -    SPICE_ROP_SET            /* 0xf    1 */
> +    SPICE_ROP_SET,           /* 0xf    1 */
> +    SPICE_ROP_END
>  } SpiceROP;
>  
>  

Frediano