Hi, Just I've built Win64 development tools, the problems you reported are completely reproduced. However, I don't have the extra licenses/machines to test Win64 binaries. I will have to ask for your help to finish this issue...
On Wed, 2 Sep 2009 18:27:22 -0400 NightStrike <[email protected]> wrote: >/home/nightstrike/work/vlc/extras/contrib/src/freetype2/src/cff/cffgload.c: >In function ‘cff_decoder_parse_charstrings’: >/home/nightstrike/work/vlc/extras/contrib/src/freetype2/src/cff/cffgload.c:866:12: >warning: cast from pointer to integer of different size [snip] >All of these are related to castings that assume that pointers are the >size of long. On this platform, they are not. Correct. Win64 is LLP64, we cannot calculate the pointer with unsigned long variables. When I've ever worked for 16-bit and 64-bit systems, I was thinking about ILP64 and LP64 systems, but I slipped to take LLP64 into account. It is my error, I'm sorry. > I looked into some of >them, and I see for instance this: > > /*************************************************************************/ > /* */ > /* <Type> */ > /* FT_Fixed */ > /* */ > /* <Description> */ > /* This type is used to store 16.16 fixed float values, like scaling */ > /* values or matrix coefficients. */ > /* */ > typedef signed long FT_Fixed; > >Well, that's an easy fix. Just change signed long to intptr_t, and >problems are solved. Yet I've not tested this, but maybe it could remove most warnings. However, I prefer to use ptrdiff_t that is available since C89, because intptr_t is available since C99. But, we cannot do such, because FT_Fixed is public interface and changing its type to different size will break the binary compatibility. >However, the comments in the typedef say that FT_Fixed is expected to >be 16 bits by 16 bits. This isn't the cast even on a linux 64 system, >where long is 32.32. Further, why is a type like this being used to >store pointers? Surely a pointer isn't a matrix coefficient. Yes, if we restrict FT_Fixed to the shortest type covering the essential part (16.16 int), the problem will occur on LP64 systems either. As you described, the proposed fix just solves the warning against invalid casts, it is not good solution. As you wrote, the interplaying between FT_Fixed and the pointer is the root of this issue, it is what should be fixed. >I want to be able to properly port this, but it seems like this isn't >as trivial as just replacing a bunch of long's with intptr_t's. There are 3 groups of the problem. A) bdflib.c In this module, the length of property name should be typed FT_Offset (= size_t) to take the return value by strlen(). On C89-conforming implementations, size_t won't exceed the unsigned long [1], so taking strlen() return value by unsigned long is ok, but it is not ok for LLP64 implementations that 64-bit size_t exceeds 32-bit unsigned long. The fix would be something like... diff --git a/src/bdf/bdf.h b/src/bdf/bdf.h index e3088a2..780168f 100644 --- a/src/bdf/bdf.h +++ b/src/bdf/bdf.h @@ -216,7 +216,7 @@ FT_BEGIN_HEADER bdf_glyph_t* unencoded; /* Unencoded glyphs themselves. */ unsigned long props_size; /* Font properties allocated. */ - unsigned long props_used; /* Font properties used. */ + size_t props_used; /* Font properties used. */ bdf_property_t* props; /* Font properties themselves. */ char* comments; /* Font comments. */ diff --git a/src/bdf/bdflib.c b/src/bdf/bdflib.c index c8afc01..54d380b 100644 --- a/src/bdf/bdflib.c +++ b/src/bdf/bdflib.c @@ -971,7 +971,7 @@ int format, bdf_font_t* font ) { - unsigned long n; + FT_Offset n; bdf_property_t* p; FT_Memory memory = font->memory; FT_Error error = BDF_Err_Ok; @@ -991,7 +991,7 @@ p = font->user_props + font->nuser_props; FT_ZERO( p ); - n = (unsigned long)( ft_strlen( name ) + 1 ); + n = ( ft_strlen( name ) + 1 ); if ( FT_NEW_ARRAY( p->name, n ) ) goto Exit; @@ -1019,7 +1019,7 @@ bdf_font_t* font ) { hashnode hn; - unsigned long propid; + FT_Offset propid; if ( name == 0 || *name == 0 ) @@ -1028,7 +1028,7 @@ if ( ( hn = hash_lookup( name, &(font->proptbl) ) ) == 0 ) return 0; - propid = (unsigned long)hn->data; + propid = (FT_Offset)hn->data; if ( propid >= _num_bdf_properties ) return font->user_props + ( propid - _num_bdf_properties ); @@ -1261,7 +1261,7 @@ char* name, char* value ) { - unsigned long propid; + FT_Offset propid; hashnode hn; bdf_property_t *prop, *fp; FT_Memory memory = font->memory; @@ -1273,7 +1273,7 @@ { /* The property already exists in the font, so simply replace */ /* the value of the property with the current value. */ - fp = font->props + (unsigned long)hn->data; + fp = font->props + (FT_Offset)hn->data; switch ( fp->format ) { @@ -1335,7 +1335,7 @@ font->props_size++; } - propid = (unsigned long)hn->data; + propid = (FT_Offset)hn->data; if ( propid >= _num_bdf_properties ) prop = font->user_props + ( propid - _num_bdf_properties ); else @@ -2044,7 +2044,7 @@ p->memory = 0; { /* setup */ - unsigned long i; + FT_Offset i; bdf_property_t* prop; @@ -2472,7 +2472,7 @@ hn = hash_lookup( name, (hashtable *)font->internal ); - return hn ? ( font->props + (unsigned long)hn->data ) : 0; + return hn ? ( font->props + (FT_Offset)hn->data ) : 0; } However, if I make a BDF font including a string longer than 32-bit length, I will find more bugs of FT2. B) t1decode.c & cffgload.c In these modules, a seed of random number to decode the encrypted Adobe Type1 & Type2 font program is calculated from the memory address of the parameters. I'm not sure which is the best to use the calculation "FT_Fixed", "FT_Int32", "FT_Long", or "FT_PtrDist". I have to check Adobe documentation. It won't be long work, please wait. C) ftcbasic.c & ftccmap.c In this module, a hash value FTC_FaceID for a FT_Face object is calculated by the memory address of the object. The hash value is designed to be 32-bit even on non 32-bit platforms. The public interface include/freetype/ftcache.h has a macro FTC_FACE_ID_HASH() that includes a cast to FT_UInt32, like this: #define FT_POINTER_TO_ULONG( p ) ( (FT_ULong)(FT_Pointer)(p) ) #define FTC_FACE_ID_HASH( i ) \ ((FT_UInt32)(( FT_POINTER_TO_ULONG( i ) >> 3 ) ^ \ ( FT_POINTER_TO_ULONG( i ) << 7 ) ) ) Even if we cannot use 64-bit hash value, we should care the overflow to avoid the hash value conflict. I guess Win64 is the first LLP64 platform that FreeType2 runs on, maybe we can insert some change if it won't break the compatibilities of non-LLP64 systems. I checked if how many FT2 client softwares use FTC_FACE_ID_HASH() macro by google, but I could not find. Fixing this (possibly the most important task) might be long work, please wait a while. Regards, mpsuzuki P.S. BTW, testing the fix for LLP64 for the overflow unsigned long by the pointer, or invalid cast from the pointer to unsigned long, I wish if there is a memory allocater returning "low" block whose address fits to 32-bit variable and another allocator returning "high" block whose address exceeds the 32-bit variable. If you know anything like that, please let me know. [1] http://coding.derkeiler.com/Archive/C_CPP/comp.lang.c/2007-03/msg02514.html I didn't know that C89-conforming ILP32 systems cannot have 64-bit size_t, even if they have 64-bit long long. _______________________________________________ Freetype mailing list [email protected] http://lists.nongnu.org/mailman/listinfo/freetype
