[PATCH 1/2] Fix "always false" conditionals

[PATCH 1/2] Fix "always false" conditionals

[przanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Enums are unsigned by default in gcc and we can't rely on any specific
signedess for the other compilers.

i965_render.c: In function ‘i965_prepare_composite’:
i965_render.c:2018:2: warning: comparison of unsigned expression < 0 is always false
i965_render.c:2025:2: warning: comparison of unsigned expression < 0 is always false
i965_render.c:2050:3: warning: comparison of unsigned expression < 0 is always false
i965_render.c:2057:3: warning: comparison of unsigned expression < 0 is always false

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 src/i965_render.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

I could also have defined FILTER_ERROR as -1, then gcc would have automagically
converted the enum to signed, but I'm not sure what other compilers would do in
this case.

diff --git a/src/i965_render.c b/src/i965_render.c
index 7e1da5b..2bd25b5 100644
--- a/src/i965_render.c
+++ b/src/i965_render.c
@@ -606,7 +606,8 @@ static const uint32_t ps_kernel_masknoca_projective_static_gen7[][4] = {
 typedef enum {
 	SAMPLER_STATE_FILTER_NEAREST,
 	SAMPLER_STATE_FILTER_BILINEAR,
-	FILTER_COUNT
+	FILTER_COUNT,
+	FILTER_ERROR
 } sampler_state_filter_t;
 
 typedef enum {
@@ -614,7 +615,8 @@ typedef enum {
 	SAMPLER_STATE_EXTEND_REPEAT,
 	SAMPLER_STATE_EXTEND_PAD,
 	SAMPLER_STATE_EXTEND_REFLECT,
-	EXTEND_COUNT
+	EXTEND_COUNT,
+	EXTEND_ERROR
 } sampler_state_extend_t;
 
 typedef enum {
@@ -1248,7 +1250,7 @@ static sampler_state_filter_t sampler_state_filter_from_picture(int filter)
 	case PictFilterBilinear:
 		return SAMPLER_STATE_FILTER_BILINEAR;
 	default:
-		return -1;
+		return FILTER_ERROR;
 	}
 }
 
@@ -1264,7 +1266,7 @@ static sampler_state_extend_t sampler_state_extend_from_picture(int repeat_type)
 	case RepeatReflect:
 		return SAMPLER_STATE_EXTEND_REFLECT;
 	default:
-		return -1;
+		return EXTEND_ERROR;
 	}
 }
 
@@ -2015,14 +2017,14 @@ i965_prepare_composite(int op, PicturePtr source_picture,
 
 	composite_op->src_filter =
 	    sampler_state_filter_from_picture(source_picture->filter);
-	if (composite_op->src_filter < 0) {
+	if (composite_op->src_filter == FILTER_ERROR) {
 		intel_debug_fallback(scrn, "Bad src filter 0x%x\n",
 				     source_picture->filter);
 		return FALSE;
 	}
 	composite_op->src_extend =
 	    sampler_state_extend_from_picture(source_picture->repeatType);
-	if (composite_op->src_extend < 0) {
+	if (composite_op->src_extend == EXTEND_ERROR) {
 		intel_debug_fallback(scrn, "Bad src repeat 0x%x\n",
 				     source_picture->repeatType);
 		return FALSE;
@@ -2047,14 +2049,14 @@ i965_prepare_composite(int op, PicturePtr source_picture,
 
 		composite_op->mask_filter =
 		    sampler_state_filter_from_picture(mask_picture->filter);
-		if (composite_op->mask_filter < 0) {
+		if (composite_op->mask_filter == FILTER_ERROR) {
 			intel_debug_fallback(scrn, "Bad mask filter 0x%x\n",
 					     mask_picture->filter);
 			return FALSE;
 		}
 		composite_op->mask_extend =
 		    sampler_state_extend_from_picture(mask_picture->repeatType);
-		if (composite_op->mask_extend < 0) {
+		if (composite_op->mask_extend == EXTEND_ERROR) {
 			intel_debug_fallback(scrn, "Bad mask repeat 0x%x\n",
 					     mask_picture->repeatType);
 			return FALSE;
-- 
1.7.4.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] Remove useless assertion

[przanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Removes 17 instances of:
warning: comparison of unsigned expression >= 0 is always true

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 src/xvmc/intel_batchbuffer.h |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/src/xvmc/intel_batchbuffer.h b/src/xvmc/intel_batchbuffer.h
index 888abeb..7fae6f7 100644
--- a/src/xvmc/intel_batchbuffer.h
+++ b/src/xvmc/intel_batchbuffer.h
@@ -44,7 +44,6 @@ extern int VERBOSE;
     do {                                                                 \
         xvmc_driver->batch.space -= (batch_ptr - xvmc_driver->batch.ptr);\
         xvmc_driver->batch.ptr = batch_ptr;                              \
-        assert(xvmc_driver->batch.space >= 0);                           \
     } while(0)
 
 extern void intelFlushBatch(Bool);
-- 
1.7.4.1

Re: [PATCH 1/2] Fix "always false" conditionals

[Chris Wilson]

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

On Fri, 16 Sep 2011 18:53:01 -0300, przanoni@gmail.com wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Enums are unsigned by default in gcc and we can't rely on any specific
> signedess for the other compilers.
> 
> i965_render.c: In function ‘i965_prepare_composite’:
> i965_render.c:2018:2: warning: comparison of unsigned expression < 0 is always false
> i965_render.c:2025:2: warning: comparison of unsigned expression < 0 is always false
> i965_render.c:2050:3: warning: comparison of unsigned expression < 0 is always false
> i965_render.c:2057:3: warning: comparison of unsigned expression < 0 is always false
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  src/i965_render.c |   18 ++++++++++--------
>  1 files changed, 10 insertions(+), 8 deletions(-)
> 
> I could also have defined FILTER_ERROR as -1, then gcc would have automagically
> converted the enum to signed, but I'm not sure what other compilers would do in
> this case.

That behaviour is guarranteed by the C standard. Besides which we only
have one compiler. ;-)

The extra verbosity is good, but you should also fixup all the switches
that use those enums (in preparation for -Wswitch-enum).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] Fix "always false" conditionals

[Chris Wilson]

On Fri, 16 Sep 2011 18:53:01 -0300, przanoni@gmail.com wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Enums are unsigned by default in gcc and we can't rely on any specific
> signedess for the other compilers.

Pushed them both after a little modification.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre