Re: [Devel] Integer issues with FreeType

[David Turner]

Hi Tom,

Tom Kacvinsky a écrit :
> 
> I apologize in advance if this offends anyone, but I really do think that 
> this will
> help us avoid problems in the future.  And I am willing to do the work, so 
> its me
> that gets to wade through all the compiler warnings and weird results that 
> will
> ensue from this change I feel is necessary.
>
I didn't feel offended. After all, constructive critiscism is hardly something
to avoid, and it doesn't happen that often on the FreeType list (not that I'm
going to start complaining a lot about that though ;-)

You and Antoine have convinced me that I should take a serious look back at
the way integers are used in FreeType, and even to my own knowledge about
them. I've started a small research for papers related to portability
issues. I would recommend the following pages to other people on the list:

    http://www.apocalypse.org/pub/u/paul/docs/cstyle/cstyle16.htm

It has, among other very interesting things, a table of integer size on
realtively different platforms. Even though the info there seems a bit
dated (for example, "shorts" can be 8 bits on a 68000 or 386 !?)

As an anedocte, I remember a very early request of porting FreeType 1
to a processor that stored _bytes_ as 32-bit integers (with 8 bits
of precision, of course), so this things still exist in the embedded
market..
 
> >
> > Why would you want to remove these, they're perfectly legal..
> >
> 
> As I said, I know they are perfectly legal, but one has no idea whatsoever 
> from
> looking at the code base whether the values being read in are 8, 16, or 32 
> bits
> wide.  Either we clarify in the headers that FT_Long is meant for reading 32 
> bit
> quantities (and hence FT_Long is a 32 bit int), FT_Short is meant fo reading 
> 16
> bit quantities (and hence should be 16 bits wide, etc...), or we use FT_intN
> intger type typedefs.
> 
> > > But what the conundrum for me is that there is no documentation of what 
> > > integer
> > > size FT_Long, FT_Int, FT_Short, etc... are supposed to be.
> > >
> > The sizes defined by the ANSI C specification, which is:
> >
> >   FT_Short  == short  == 16-bits
> >   FT_Int    == int    == at least 16 bits, can be more (i.e. 24 bits, 32 
> > bits, etc..)
> >   FT_Long   == long   == at least 32 bits, can be more (i.e. 36 bits, 64 
> > bits, etc..)
> >
> 
> Yes, I know what the sizes are from the ANSI specification.  My question was
> (and still is) this: from a *FreeType* standpoint, what is FT_Long supposed to
> be used for, and how wide is it supposed to be ?  Now, I know from experience 
> in
> the code base that FT_Long is used for reading in or storing 32 bit 
> quantities,
> but that isn't in the documentation or the header files, is it?  This isn't
> strictly for me and my selfish goals, it is for others. :)
>

 
> > You should use the FT_IntXX and FT_UIntXX types only when you
> > absolutely need an integer of an exact size. This is generally
> > used when you're dealing with the structure of integers in
> > memory, or when you need to perform exact wrapping, like in
> > "((FT_UInt16)(x + 16))"..
> >
> 
> This is where I believe I have disagreed with you in the past. :) I believe 
> that
> one should *always* use the exact size needed.  Otherwise, we get into
> unnecessary bitwise operations.  Unnecessary in the sense that we don't have 
> to
> these kinds of things when storing a 32 bit signed quantity in 64 bit integer:
> 
>   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 believe that using explicit cast is the only safe way, because
C promotion rules can be so confusing. Just to give you another
example:

      FT_Byte     c = 125;
      FT_UInt16   s = 2000;

      if ( c < s )
      {
        ...
      }

with Borland C++, which is one of the most pedantic ANSI compiler I've found,
the byte is promoted to an int in the comparison, and the compiler warns about
"comparing signed and unsigned"..

> > >
> > FT_Int isn't supposed to be a 16-bits int. You should not use it to
> > perform arithmetic on 1--bit values however, unless you take care of
> > casting everything appropriately..
> >
> 
> Again, I am being pedantic, but the fact that FT_Int is not supposed to be 16
> bits wide is not documented in fttypes.h.
>
fttypes.h simply says that "FT_Int" is a convenient typedefs for "int",
which means that it has the exact same properties.

 
> > No, no, that's not correct, you're going to introduce lots of problem in the
> > source code if you do so, not mentioning adding about 100K compilation 
> > warnings :-)
> >
> 
> OK, so now I know that FT_* integer types are not based on the TT spec.  Help 
> me
> understand the basis for using FT_Long, then.  Is it because the C89 spec. 
> says
> the int type needs to be at least 16 bits wide, whereas the long type is
> guaranteed to be at least 32 bits wide, and we have need to read in 32 it
> quantities?
>
I suppose that the use of FT_Long to store 32-bit quantities is historical
(FT_Int32 was introduced rather recently in the history of FT2) and maybe
discarded in favor of using the correct data types.

However, I fear that changing the meaning of FT_Int and FT_Long is going
to introduce its own set of delicious bugs. I'd advocate the creation of
new types like FT_Fast16 and FT_Fast32 when "fast integers" are needed,
and "FT_Int32" and "FT_Int16" regarding storage..

> 
> I didn't meant that there were bugs in ftcalc.c.  I meant that if I change 
> what FT_Long
> menas, then there will be some work to do ftcalc.c.
>
OK, it seems indeed that "ftcalc.c" assumes that "FT_Long" is 32-bits when
a 64-bits integer type was detected..
 
> As for the portability issue: there is some work that needs to be done. I 
> don't
> consider code that core dumps (or gives erroneous results) on a machine that 
> has
> 64 bit longs portable.  In fact, I have a complaint that my item in the TODO
> list specifiying that 64 bit long issues needed to be addressed was removed.
>
Sorry, I've probably been a bit quick to remove this item, I just put it back..
 
> >
> > We could improve the FreeType source code slightly to detect 64-bit integers
> > on a few compilers that support them like gcc, visual c++, etc.., without
> > using Autoconf if you like, but this doesn't need the deep inspection you're
> > mentioning..
> >
> 
> Well, lets here it!
>
I have commited some changes to "ftconfig.h" and "ftoption.h" yesterday.
Basically, "ftconfig.h" checks the compiler to see if a 64-bit type it
knows about is available, while "ftoption.h" defines the
FT_CONFIG_OPTION_FORCE_INT64 configuration macro. Setting it will force
the use of the 64-bit type, even when compiling in ANSI-C mode
(this will result in compiler warnings but the code will run).

The changes are not perfect, one needs to:

   - add more static compiler checks (fortunately, we really only need to
     consider compilers on platforms where Autoconf is not available)

   - test, test, test..
 
> >
> > Tom, I think you're heading for trouble.. What really needs to be checked
> > are:
> >
> >   - arithmetic computations with 16-bit and 32-bit quantities
> >   - use of pointer differences
> >   - casts (to see if they're applied appropriately)
> >
> 
> I would add that assigning 32 bit signed quantities to 64 bit signed ints 
> needs
> to be addressed.
>
OK.

Regards,

- David