Paul Messmer <
address@hidden> wrote:
>Testing after removing the factor of 2 isn't showing any problems for me.
>
>-- Paul
>
>On Fri, Feb 5, 2010 at 3:48 PM, Paul Messmer <
address@hidden> wrote:
>
>> The overallocation is an urgent issue for me, but requires no urgent action
>> on your part. The bar for me to make a small local fix to FreeType is lower
>> than integrating an entirely new library release. After looking at the code
>> further, I'm less concerned about the possibility of access of more than
>> n_points FT_Vectors, and I also have some testing I can exercise to give me
>> some confidence the "fix" doesn't break anything... at least in the scope of
>> my usage of FreeType.
>>
>> I wanted to bring this up with freetype-devel in order to get a second
>> opinion about the situation, and so the problem could be fixed in your
>> releases going forward. As there doesn't appear to be a good test suite for
>> FreeType, your exercising caution with respect to the change is a fine idea
>> as over-allocation can easily hide a latent and non-obvious bug.
>>
>> I'll keep you up to date on whether I encounter any failures after removing
>> the factor of 2 in the allocation. Thanks.
>>
>> -- Paul
>>
>> On Thu, Feb 4, 2010 at 10:35 PM, <
address@hidden> wrote:
>>
>>> Dear Paul,
>>>
>>> On Thu, 4 Feb 2010 15:29:28 -0800
>>> Paul Messmer <
address@hidden> wrote:
>>> >The code above seems to believe there are n_points FT_Vectors allocated.
>>> > However,
>>> >
>>> >.../base/ftoutln.c: FT_Outline_New_Internal()
>>> >
>>> > if ( FT_NEW_ARRAY( anoutline->points, numPoints * 2L ) ||
>>> > FT_NEW_ARRAY( anoutline->tags, numPoints ) ||
>>> > FT_NEW_ARRAY( anoutline->contours, numContours ) )
>>> > goto Fail;
>>> > anoutline->n_points = (FT_UShort)numPoints;
>>> >
>>> >This seems to be allocating 2*n_points FT_Vectors, so there's a
>>> difference
>>> >between how much memory is actually being used and how much it believes
>>> is
>>> >being used.
>>>
>>> Great thank you for finding the problem.
>>>
>>> Tracking the history of the ftoutln.c, I think it came
>>> from freetype-1 era (!). A "point" in TT_Outline was
>>> described by TT_Vector, like this:
>>>
>>>
>>>
http://cvs.savannah.gnu.org/viewvc/freetype/lib/freetype.h?revision=1.72&root=freetype&view=markup
>>>
>>> struct TT_Outline_
>>> {
>>> TT_Short n_contours; /* number of contours in glyph */
>>> TT_UShort n_points; /* number of points in the glyph */
>>>
>>> TT_Vector* points; /* the outline's points */
>>> ...
>>>
>>> But TT_New_Outline() allocated the buffer as the twice
>>> of sizeof ( TT_F26Dot6 ), like this:
>>>
>>>
>>>
http://cvs.savannah.gnu.org/viewvc/freetype/lib/ttapi.c?revision=1.55&root=freetype&view=markup
>>>
>>> FT_EXPORT_FUNC( TT_Error )
>>> TT_New_Outline( TT_UShort numPoints,
>>> TT_Short numContours,
>>> TT_Outline* outline )
>>> {
>>> ...
>>>
>>> if ( ALLOC( outline->points, numPoints*2*sizeof ( TT_F26Dot6 ) ) ||
>>> ALLOC( outline->flags, numPoints *sizeof ( Byte ) ) ||
>>> ALLOC( outline->contours, numContours*sizeof ( UShort ) ) )
>>> goto Fail;
>>>
>>> If it were written "sizeof ( FT_Vector )", this problem might not
>>> have occured. This "2" was carried over to FreeType2, because the
>>> initial version of FreeType2 still specified its size by twice of
>>> sizeof( FT_Pos ), like this:
>>>
>>>
>>>
http://cvs.savannah.gnu.org/viewvc/freetype2/src/base/ftoutln.c?revision=1.1&root=freetype&view=markup
>>>
>>> if ( ALLOC_ARRAY( outline->points, numPoints * 2L, FT_Pos ) ||
>>> ALLOC_ARRAY( outline->flags, numPoints, FT_Byte ) ||
>>> ALLOC_ARRAY( outline->contours, numContours, FT_UShort ) )
>>> goto Fail;
>>>
>>> Since freetype-2.1.0, FreeType2 uses FT_NEW_ARRAY() which
>>> automatically calculate the size of array element with
>>> sizeof( *first_arg ), so the factor 2 is not needed anymore,
>>> as you've pointed out. So, this problem has long life since
>>> freetype-2.1.0. The moment how overallocation can be found
>>> at:
>>>
>>>
>>>
http://cvs.savannah.gnu.org/viewvc/freetype2/src/base/ftoutln.c?root=freetype&r1=1.44&r2=1.45
>>>
>>> --
>>>
>>> I want to remove this extra factor "2". But you also mentioned
>>> the possibility that the buffer over outline->n_points is used
>>> in some special case, so I hesitate to remove it without careful
>>> check. This over allocation is urgent issue for you? If not, I
>>> want to work for this issue after next official release (expected soon).
>>>
>>> Thank you again for detailed investigation and comment.
>>>
>>> Regards,
>>> mpsuzuki
>>>
>>
>>
>