Bug 125750 - x11/xloadimage LP64 bugs
Summary: x11/xloadimage LP64 bugs
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: jmz
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-07-18 16:00 UTC by Christian Weisgerber
Modified: 2008-07-20 17:50 UTC (History)
0 users

See Also:


Attachments
file.txt (3.66 KB, text/plain)
2008-07-18 16:00 UTC, Christian Weisgerber
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Weisgerber freebsd_committer 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 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 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 2008-07-20 17:45:46 UTC
State Changed
From-To: open->closed

Committed. Thanks!