2007.09.12 03:36 "[Tiff] 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:

• Require that SIZEOF_UNSIGNED_LONG be explicitly 8 or 4, and if it is not defined we #error to stop the build.
• 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