From naddy@openbsd.org Tue Jul  1 22:06:00 2008
Date: Tue, 1 Jul 2008 22:06:00 +0200
From: Christian Weisgerber <naddy@openbsd.org>
To: James Troup <james@nocrew.org>
Cc: jmz@FreeBSD.org
Subject: xloadimage 4.1-16: LP64 bugs
Message-ID: <20080701200600.GA39658@kemoauc.mips.inka.de>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.4.2.3i
Status: RO
Content-Length: 4681
Lines: 135

Dear Debian and FreeBSD port maintainers,

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

I have appended a patch below.  Please review, and if you find the
fixes correct, apply them to xloadimage in your projects.

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);
--- 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))))))