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 <[email protected]> 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, <[email protected]> wrote: > >> Dear Paul, >> >> On Thu, 4 Feb 2010 15:29:28 -0800 >> Paul Messmer <[email protected]> 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 >> > >
_______________________________________________ Freetype-devel mailing list [email protected] http://lists.nongnu.org/mailman/listinfo/freetype-devel
