Re: [Devel] Integer issues with FreeType

[Tom Kacvinsky]

Hi David,

>
> >
> > > >
> > > >   i = (j << 32) >> 32.
> > > >
> > >
> > > This is a degenerate example, what about:
> > >
> > >     i = (j << 31) >> 16;
> > >
> > > The meaning of this expression varies with the sign of the variables and 
> > > can
> > > lead to potential bugs. You don't need only the exact size, but the exact 
> > > sign
> > > when bit-shifting..
> > >
> >
> > I don't think you need the exact sign when bit shifting.  If the 32 bit 
> > quantity
> > read in is supposed to be a signed int, but is positive, (sign bit is not 
> > set),
> >
> >     i = (j << 32) >> 32
> >
> > will still result in the correct value.  But I agree, one needs to know the
> > exact integer width before one starts playing bit shifting games.
>
> The problem is that your example can be read in two different ways if
> you don't know the context:
>
>   - if "j" is assumed to be a 32-bit int, the result of "i" will always be 0
>

I meant that i should be a 32 bit unsigned int, j a 64 bit signed int.

I see that on Tru64 UNIX, the compiler does not complain, and the result is
always 0.  On Solaris, the compiler complains about bit shifting on the 32 bit
quantity, but the result is not zero: it is the value of as if i were stuffed
into j without sign extension.  Hmmmm both compilers were in ANSI mode.  Which
compiler is correct?  Thanks for correcting the errors of my ways...0

>   - if "j" is a 64-bit FT_Long (on Tru64 for example), then the value of
>     "i" is the sign extension of the 32-bit quantity held in "j"
>

This is what I was after -- sign extension.  But I see now that the explicit
cast is necessary, at least on SunOS 7, using the Forte C compiler in 64 bit
mode.  And also on Tru64 UNIX with Compaq's C compiler.  Both compilers in
ANSI mode.

> This ambiguity doesn't exist if you use explicit casts, like in:
>
>        i = ((FT_Int64)j << 32) >> 32;
>
> the explicit cast of "j" is free (it doesn't result in additional code),
> however, it makes it very clear both to the compiler and developer what's
> the statement's meaning is..
>
> Similary, "i = (j << 31) >> 16", can be replaced by one of the following:
>
>     i =  (FT_Int32)(j << 31)  >> 16;
>     i = (FT_UInt32)(j << 31) >> 16;
>
> these are respectively equivalent to:
>
>     i = (j & 1) ? 0xFFFF0000  : 0;
>     i = (j & 1) ? 0x00010000U : 0;
>
> If i'm not myself completely mistaken :-)
>

I did not get those results.  Assuming j is a 32 bit unsigned quantity and i is
a 32 bit signed quantity, as in:

#include <stdio.h>

int main () {

  unsigned int i = 0xFFFFFFFF;
  int j;

  j = ((unsigned int)i << 31) >> 16;

  printf ("i = %d, j = 0x%08X.\n", i, j );

  j = ((int)i << 31) >> 16;

  printf ("i = %d, j = 0x%08X.\n", i, j );

  return (0);

}

I get these results:

i = -1, j = 0x00008000.
i = -1, j = 0xFFFF8000.

> I still think that explicit casts are a much needed _safety_ precaution
> when right-shifts (>>) are used..
>

Right, I agree now. :)

> > There is a
> > way to sign extend a 32 bit quantity and stuff it into a N ( > 32 ) bit 
> > quantity
> > without knowing the exact integer width (one just needs to know the width is
> > greater tha 32 bits).  I have to look it up though.  It is in the t1utils 
> > code
> > (now maintained by Ed Kohler at MIT).
> >
>
> That could be intersting, I'd like to see that.. :-)
>

Here it is, from the t1utils code (t1disasam.c):

        /* in case an int32 is larger than four bytes---sign extend */
#if INT_MAX > 0x7FFFFFFFUL
        if (val & 0x80000000)
          val |= ~0x7FFFFFFF;
#endif

Assuming the value of val occupies 32 bits of data but val can hold more than 32
bits of data , this will make sure val gets sign extended.  I imagine this code
will work when val is a 64 bit signed int, but the values stored in val are
unsigned (as in reading from a byte stream) and occupy no more than 32 bits of
data.

>
> > That example was meant to illustrate taking a signed 32 int and stuffing it 
> > into
> > a signed 64 bit int.  Unless I got the wrong compiler flags (to force the
> > compiler in ANSI compliant mode), this is necessary to sign extend when 
> > storing.
> > I have to check what problems I ran into with casting from an unsigned 
> > quantity
> > to a signed quantity, but I believe explict casting didn't always work.
> >
> > NOTE: I am not oppsoed to explicit casting when it is necessary.
> >
>
> Even when it's not necessary, it makes the code clearer and avoids strange
> ambiguities. Think about the occasional hacker who doesn't even know that
> the code can run on a 64-bit system, and thinks that FT_Long is 32-bit..
>
> One more reason to get rid of FT_Long usage, and use expressive casts..
>

I agree with both points...

Tom