AWARE [SYSTEMS] Imaging expertise for the Delphi developer
AWare Systems, Imaging expertise for the Delphi developer, Home TIFF and LibTiff Mailing List Archive

LibTiff Mailing List

TIFF and LibTiff Mailing List Archive
September 2007

Previous Thread
Next Thread

Previous by Thread
Next by Thread

Previous by Date
Next by Date

Contact

The TIFF Mailing List Homepage
This list is run by Frank Warmerdam
Archive maintained by AWare Systems



Valid HTML 4.01!



Thread

2007.09.12 03:36 "SIZEOF_UNSIGNED_LONG", by Frank Warmerdam
2007.09.12 12:59 "Re: SIZEOF_UNSIGNED_LONG", by Edward Lam
2007.09.12 13:04 "Re: SIZEOF_UNSIGNED_LONG", by Edward Lam
2007.09.12 13:56 "Re: SIZEOF_UNSIGNED_LONG", by Frank Warmerdam
2007.09.12 14:09 "Re: SIZEOF_UNSIGNED_LONG", by Edward Lam
2007.09.12 15:01 "Re: SIZEOF_UNSIGNED_LONG", by Frank Warmerdam
2007.09.12 15:09 "Re: SIZEOF_UNSIGNED_LONG", by Edward Lam
2007.09.12 15:02 "Re: SIZEOF_UNSIGNED_LONG", by Bob Friesenhahn

2007.09.12 03:36 "SIZEOF_UNSIGNED_LONG", by Frank Warmerdam

Folks,

libtiff now (I'm not clear on how recent this is) checks
SIZEOF_UNSIGNED_LONG in tif_fax3.c and has an alternate form
of the FILL() macro for 64bit systems.  This introduced a bug in
GDAL which didn't declare the SIZEOF_UNSIGNED_LONG macro when doing
"libtiff internal" builds on 64bit systems.

The code looks like:

#if SIZEOF_UNSIGNED_LONG == 8
# define FILL(n, cp)							    \
     switch (n) {							    \
     case 15:(cp)[14] = 0xff; case 14:(cp)[13] = 0xff; case 13: (cp)[12] = 0xff;\
     case 12:(cp)[11] = 0xff; case 11:(cp)[10] = 0xff; case 10: (cp)[9] = 0xff;\
     case  9: (cp)[8] = 0xff; case  8: (cp)[7] = 0xff; case  7: (cp)[6] = 0xff;\
     case  6: (cp)[5] = 0xff; case  5: (cp)[4] = 0xff; case  4: (cp)[3] = 0xff;\
     case  3: (cp)[2] = 0xff; case  2: (cp)[1] = 0xff;			      \
     case  1: (cp)[0] = 0xff; (cp) += (n); case 0:  ;			      \
     }
...
#else
# define FILL(n, cp)							    \
     switch (n) {							    \
     case 7: (cp)[6] = 0xff; case 6: (cp)[5] = 0xff; case 5: (cp)[4] = 0xff; \
     case 4: (cp)[3] = 0xff; case 3: (cp)[2] = 0xff; case 2: (cp)[1] = 0xff; \
     case 1: (cp)[0] = 0xff; (cp) += (n); case 0:  ;			    \
     }
...
#endif

The concern I have is that this code defaults to assuming 32bit if the
macro is not defined, and it produces incorrect results if the unsigned
long is actually 64bit.

I wonder if we should either:
  o Require that SIZEOF_UNSIGNED_LONG be explicitly 8 or 4, and if it is not
    defined we #error to stop the build.
  o Add some sort of runtime checking to detect that sizeof(unsigned long) is
    actually 4 if SIZEOF_UNSIGNED_LONG is not defined.

It would make things somewhat less fragile (with hard to discover errors).

Best regards,
-- 
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam, warmerdam@pobox.com
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | President OSGeo, http://osgeo.org