Bug 125750

Summary: x11/xloadimage LP64 bugs
Product: Ports & Packages Reporter: Christian Weisgerber <naddy>
Component: Individual Port(s)Assignee: jmz
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
file.txt none

Description Christian Weisgerber freebsd_committer freebsd_triage 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:
Comment 1 Edwin Groothuis freebsd_committer freebsd_triage 2008-07-18 16:00:18 UTC
Responsible Changed
From-To: freebsd-ports-bugs->jmz

Over to maintainer (via the GNATS Auto Assign Tool)
Comment 2 dfilter service freebsd_committer freebsd_triage 2008-07-20 17:45:42 UTC
jmz         2008-07-20 16:45:28 UTC

  FreeBSD ports repository

  Modified files:
    x11/xloadimage       Makefile 
  Added files:
    x11/xloadimage/files patch-lp64 
  Log:
  Fix builds on 64-bit platforms
  
  PR:             ports/125750
  Submitted by:   naddy
  
  Revision  Changes    Path
  1.36      +1 -1      ports/x11/xloadimage/Makefile
  1.1       +76 -0     ports/x11/xloadimage/files/patch-lp64 (new)
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
Comment 3 jmz freebsd_committer freebsd_triage 2008-07-20 17:45:46 UTC
State Changed
From-To: open->closed

Committed. Thanks!