[ft-devel] [PATCH] Portability to platforms where >> is logical (unsigned) shift

[Jon Foster]

Hi,

This patch fixes a portability problem in FreeType.  It also adds a
big comment explaining what the code's trying to do (which was not
obvious to me), and what the portability problem is.  So I won't
repeat myself here.

This patch is against 2.3.5, but looks like it should apply against
CVS head.

There may be a slight performance drop from applying this patch.
(But I haven't benchmarked it).  So if the original implementation
is known to work on certain platforms, it might be worth adding
ifdefs to detect those platforms at compile-time.

Kind regards,

Jon Foster

diff -upw freetype-2.3.5/src/base/ftcalc.c
modified/freetype-2.3.5/src/base/ftcalc.c
--- freetype-2.3.5/src/base/ftcalc.c    2007-06-11 06:51:49.000000000
+0100
+++ fourth/freetype-2.3.5/src/base/ftcalc.c     2008-07-16
17:54:18.059417500 +0100
@@ -436,7 +436,12 @@
     );
     return result;

-#elif 1
+#elif 0
+    /* This code is nonportable.  See comment below.
+     *
+     * However, on a platform where right-shift of a signed quantity
+     * fills the leftmost bits by copying the sign bit, it might be
faster.
+     */

     FT_Long   sa, sb;
     FT_ULong  ua, ub;
@@ -445,6 +450,22 @@
     if ( a == 0 || b == 0x10000L )
       return a;

+    /* This is a clever way of converting a signed number "a" into
+     * it's absolute value (stored back into "a") and it's sign.  The
sign
+     * is stored in "sa"; 0 means "a" was positive or zero, and -1
means
+     * "a" was negative.  (Similarly for "b" and "sb").
+     *
+     * Unfortunately, it doesn't work (at least not portably).
+     *
+     * It makes the assumption that right-shift on a negative signed
value
+     * fills the leftmost bits by copying the sign bit.  This is wrong.
+     * According to K&R 2nd ed, section "A7.8 Shift Operators" on page
206,
+     * the result of right-shift of a negative signed value is
+     * implementation-defined.  At least one implementation fills the
leftmost
+     * bits with 0s (i.e. it's exactly the same as an unsigned right
shift).
+     * This means that when "a" is negative, "sa" ends up with the
value 1
+     * rather than -1.  After that, everything else goes wrong.
+     */
     sa = ( a >> ( sizeof ( a ) * 8 - 1 ) );
     a  = ( a ^ sa ) - sa;
     sb = ( b >> ( sizeof ( b ) * 8 - 1 ) );



**********************************************************************
This email and its attachments may be confidential and are intended solely for 
the use of the individual to whom it is addressed. Any views or opinions 
expressed are solely those of the author and do not necessarily represent those 
of Cabot Communications Ltd.

If you are not the intended recipient of this email and its attachments, you 
must take no action based upon them, nor must you copy or show them to anyone.

Cabot Communications Limited
Verona House, Filwood Road, Bristol BS16 3RY, UK
+44 (0) 1179584232

Co. Registered in England number 02817269

Please contact the sender if you believe you have received this email in error.

**********************************************************************


______________________________________________________________________
This email has been scanned by the MessageLabs Email Security System.
For more information please visit http://www.messagelabs.com/email 
______________________________________________________________________