[ft-devel] endian issue in ftmac.c

[mpsuzuki]

Hi,

2 weeks ago, I received a report from Mr. David Sachitano:
ftmac.c is dependent with system endian and does not
work on Intel Mac. Afterwards, he sent me a patch written
by Mr. Apple Lawrence Coopet (from Apple). His patch uses
CoreFoundation functions to fix endian issue, e.g.
CFSwapInt16BigToHost(). Of course, they are unavailable in
MPW environment, so I replaced them by macro functions.
Attached patch is my modified version.

# I have to apologize. My indepth rewrite of ftmac.c was
# a fix for the legacy functions which are deprecated for
# Intel Mac. But I didn't have Intel Mac at that time and
# slipped to prepair the expected endian issue.

I've tested patched ftmac.c on Intel Mac, and checked
that the outputs by ft2demos' ftoldmac are exactly same
between PowerPC binary (executed via Rosetta emulation)
and Intel native binary. So, I think, the patch is not
perfect solution (some internal data is still stored in
reverse byte-order), but it can work quick fix of endian
issue, for public API. Without the patch, almost functions
of ftmac.c causes SEGV crashes, because its core part
(FOND parser) has endian issue.  

However, the coding style of ftmac.c is quite different from
official FreeType2. The patch fixes the endian issue of the
platforms without memory alignments: m68k, PowerPC and x86,
but the type declaration by raw "short" "long" should be
removed (although I don't think there is any MacOS on MIPS).
I'm thinking of rewriting ftmac.c to fit the official coding
style of FreeType2. But it spends more time, so I suppose
the quick fix is expected, until the indepth rewrite of ftmac.c.

David, Werner, how do you think of applying the patch to CVS?

Regards,
mpsuzuki


--- freetype2--official-maintrunk--0.2--patch-111/src/base/ftmac.c      
2006-06-20 17:09:13.000000000 +0900
+++ freetype2/src/base/ftmac.c  2006-06-20 17:01:27.000000000 +0900
@@ -64,6 +64,7 @@
   /* This is for Mac OS X.  Without redefinition, OS_INLINE */
   /* expands to `static inline' which doesn't survive the   */
   /* -ansi compilation flag of GCC.                         */
+#undef  OS_INLINE
 #define OS_INLINE   static __inline__
 #include <Carbon/Carbon.h>
 #else
@@ -74,6 +75,20 @@
 #include <TextUtils.h>
 #endif
 
+/*
+ * XXX: convertors for byte-order issue on Intel Mac.
+ */
+#if defined( TARGET_RT_LITTLE_ENDIAN ) && ( TARGET_RT_LITTLE_ENDIAN > 0 )
+#define FT_INT16BE_HOST( s ) \
+  ( ( ( ( s ) <<  8 ) & 0xFF00 ) + ( ( ( s ) >> 8 ) & 0x00FF ) )
+#define FT_INT32BE_HOST( l ) \
+  ( ( ( ( l ) << 24 ) & 0xFF000000 ) + ( ( ( l ) <<  8 ) & 0x00FF0000 ) \
+  + ( ( ( l ) >>  8 ) & 0x0000FF00 ) + ( ( ( l ) >> 24 ) & 0x000000FF ) )
+#else
+#define FT_INT16BE_HOST( s ) ( s )
+#define FT_INT32BE_HOST( l ) ( l )
+#endif
+
 #if defined( __MWERKS__ ) && !TARGET_RT_MAC_MACHO
 #include <FSp_fopen.h>
 #endif
@@ -536,7 +551,7 @@
     /* The count is 1 greater than the value in the FOND.  */
     /* Isn't that cute? :-)                                */
 
-    return 1 + *( (short*)( fond_data + sizeof ( FamRec ) ) );
+    return FT_INT16BE_HOST( *( (short*)( fond_data + sizeof ( FamRec ) ) ) ) + 
1;
   }
 
 
@@ -549,13 +564,13 @@
 
 
     fond     = (FamRec*)fond_data;
-    face_all = *( (short *)( fond_data + sizeof ( FamRec ) ) ) + 1;
+    face_all = FT_INT16BE_HOST( *( (short *)( fond_data + sizeof ( FamRec ) ) 
) ) + 1;
     assoc    = (AsscEntry*)( fond_data + sizeof ( FamRec ) + 2 );
     face     = 0;
 
     for ( i = 0; i < face_all; i++ )
     {
-      if ( 0 == assoc[i].fontSize )
+      if ( 0 == FT_INT16BE_HOST( assoc[i].fontSize ) )
         face++;
     }
     return face;
@@ -597,19 +612,19 @@
 
       /* if the face at this index is not scalable,
          fall back to the first one (old behavior) */
-      if ( assoc->fontSize == 0 )
+      if ( FT_INT16BE_HOST( assoc->fontSize ) == 0 )
       {
         *have_sfnt = 1;
-        *sfnt_id   = assoc->fontID;
+        *sfnt_id   = FT_INT16BE_HOST( assoc->fontID );
       }
       else if ( base_assoc->fontSize == 0 )
       {
         *have_sfnt = 1;
-        *sfnt_id   = base_assoc->fontID;
+        *sfnt_id   = FT_INT16BE_HOST( base_assoc->fontID );
       }
     }
 
-    if ( fond->ffStylOff )
+    if ( FT_INT32BE_HOST( fond->ffStylOff ) )
     {
       unsigned char*  p = (unsigned char*)fond_data;
       StyleTable*     style;
@@ -619,10 +634,10 @@
       int             i;
 
 
-      p += fond->ffStylOff;
+      p += FT_INT32BE_HOST( fond->ffStylOff );
       style = (StyleTable*)p;
       p += sizeof ( StyleTable );
-      string_count = *(unsigned short*)(p);
+      string_count = FT_INT16BE_HOST( *(unsigned short*)(p) );
       p += sizeof ( short );
 
       for ( i = 0; i < string_count && i < 64; i++ )
@@ -770,13 +785,13 @@
     Str255    lwfn_file_name;
     UInt8     buff[HFS_MAXPATHLEN];
     FT_Error  err;
+    short     num_faces;
 
 
     have_sfnt = have_lwfn = 0;
 
     HLock( fond );
     parse_fond( *fond, &have_sfnt, &sfnt_id, lwfn_file_name, 0 );
-    HUnlock( fond );
 
     if ( lwfn_file_name[0] )
     {
@@ -787,9 +802,12 @@
     }
 
     if ( have_lwfn && ( !have_sfnt || PREFER_LWFN ) )
-      return 1;
+      num_faces = 1;
     else
-      return count_faces_scalable( *fond );
+      num_faces = count_faces_scalable( *fond );
+
+    HUnlock( fond );
+    return num_faces;
   }