Description Christian Weisgerber 2008-07-18 16:00:08 UTC

I have found a number of bugs in xloadimage 4.1-16 while examining these
warnings produced by gcc when building on 64-bit platforms:

cmuwmraster.c: In function `cmuwmIdent':
cmuwmraster.c:51: warning: comparison is always true due to limited range of data type
cmuwmraster.c: In function `cmuwmLoad':
cmuwmraster.c:94: warning: comparison is always true due to limited range of data type
cmuwmraster.c:111: warning: cast from pointer to integer of different size

Issue #1
--------
The final warning is straightfoward:

          fprintf(stderr,"CMU WM raster %s is of depth %d, must be 1",
                  name,
                  (int) header.depth); 
                
This doesn't make sense, header.depth is an array, and the third 
argument should really be memToVal(header.depth, 2).

Issue #2
--------
The length parameter to the memToVal(ptr, len) macro is always given
as sizeof(type).  This is pretty silly for accessing fields in
struct cmuwm_headerm, since these are defined as byte arrays.  It
is also outright wrong in the case of "sizeof(long)" when a length
of 4 is intended.

Issue #3
--------
The "comparison is always true" warnings.  Bear with me, this is a
tricky case of C integer type promotion.  If you examine the
memToVal() macro closely for the len==4 case, the byte combining
arithmetic happens at the default type, which is int.  The result
is then cast to unsigned long.  The line

  if (memToVal(header.magic, 4) != CMUWM_MAGIC)

really is something like

  if ((unsigned long)(int expression) != 0xf10040bb)

Even if the int expression evaluates to 0xf10040bb, on a LP64
platform the cast will then cause it to be sign extended (yes!) to
0xfffffffff10040bb before the comparison happens, so the left and
right side will never compare as equal.

The simplest fix is to pull the cast into the parenthesis so that
the arithmetic expression already happens at type unsigned long.
This is in fact already the case for len==2 and len==3.

Fix: Patch attached with submission follows: