I have found a number of problems in an obscure corner of x11/xloadimage
while examining these warnings:
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
I have appended a patch below. Please review. Here is a discussion
of the problems:
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.
--- cmuwmraster.c.orig Tue Jul 1 19:08:24 2008
+++ cmuwmraster.c Tue Jul 1 19:08:57 2008
@@ -22,9 +22,9 @@ struct cmuwm_header *headerp;
{
printf("%s is a %ldx%ld %ld plane CMU WM raster\n",
name,
- memToVal(headerp->width, sizeof(long)),
- memToVal(headerp->height, sizeof(long)),
- memToVal(headerp->depth, sizeof(short)));
+ memToVal(headerp->width, 4),
+ memToVal(headerp->height, 4),
+ memToVal(headerp->depth, 2));
}
int cmuwmIdent(fullname, name)
@@ -48,7 +48,7 @@ char *fullname, *name;
break;
case sizeof(struct cmuwm_header):
- if (memToVal(header.magic, sizeof(long)) != CMUWM_MAGIC)
+ if (memToVal(header.magic, 4) != CMUWM_MAGIC)
{
r = 0;
break;
@@ -91,7 +91,7 @@ unsigned int verbose;
exit(1);
case sizeof(struct cmuwm_header):
- if (memToVal(header.magic, sizeof(long)) != CMUWM_MAGIC)
+ if (memToVal(header.magic, 4) != CMUWM_MAGIC)
{
zclose(zf);
return(NULL);
@@ -104,16 +104,16 @@ unsigned int verbose;
return(NULL);
}
- if (memToVal(header.depth, sizeof(short)) != 1)
+ if (memToVal(header.depth, 2) != 1)
{
fprintf(stderr,"CMU WM raster %s is of depth %d, must be 1",
name,
- (int) header.depth);
+ memToVal(header.depth, 2));
return(NULL);
}
- image = newBitImage(width = memToVal(header.width, sizeof(long)),
- height = memToVal(header.height, sizeof(long)));
+ image = newBitImage(width = memToVal(header.width, 4),
+ height = memToVal(header.height, 4));
linelen = (width / 8) + (width % 8 ? 1 : 0);
lineptr = image->data;
--- image.h.orig Tue Jul 1 21:18:52 2008
+++ image.h Tue Jul 1 21:21:24 2008
@@ -163,7 +163,7 @@ typedef struct {
((LEN) == 2 ? ((unsigned long) \
(*(byte *)(PTR) << 8) | \
(*((byte *)(PTR) + 1))) : \
- ((unsigned long)((*(byte *)(PTR) << 24) | \
+ (((unsigned long)(*(byte *)(PTR) << 24) | \
(*((byte *)(PTR) + 1) << 16) | \
(*((byte *)(PTR) + 2) << 8) | \
(*((byte *)(PTR) + 3)))))))
@@ -176,7 +176,7 @@ typedef struct {
(*((byte *)(PTR) + 2) << 16)) : \
((LEN) == 2 ? ((unsigned long) \
(*(byte *)(PTR)) | (*((byte *)(PTR) + 1) << 8)) : \
- ((unsigned long)((*(byte *)(PTR)) | \
+ (((unsigned long)(*(byte *)(PTR)) | \
(*((byte *)(PTR) + 1) << 8) | \
(*((byte *)(PTR) + 2) << 16) | \
(*((byte *)(PTR) + 3) << 24))))))
--
Christian "naddy" Weisgerber [EMAIL PROTECTED]
