Re: [Devel] Possible bug in ftxpost.c ?

[Jean-Christophe Dubacq]

On Mon, Jul 08, 2002 at 05:30:48PM +0200, Antoine Leca wrote:
> What you do is again to "populate" the  post20->glyphNames  array as
> early as possible, but while storing at the correct index. I agree this
> fixes the problem with Impact (and similarly "improved" fonts).
> However, it still fails with some badly behaving fonts, such as those
> where a PSname occurs multiple times (more than one glyph refers to the
> same name! and I know them to exist, even if I do not have one at hand
> with the problem occuring outside the standard names): in such a case,
> we still could encounter a overrun problem with your patch.

Yes. Obviously, one has to count the max index, not use the number of
glyphs with names.

> > I checked freetype2, and the fix is almost exactly the same. There is a
> > trick a bit more complicated to use the least possible memory, but it
> > didn't matter muchi (equivalent function is in src/sfnt/ttpost.c).
> 
> Did you really check? It appears to me that Freetype 2 has the problem
> already fixed (probably as part of the "enhancement" to spare memory).

Misunderstanding (or more probably miswriting from me): I meant to say that
it was already fixed in FT2, and that the way it was coded looked almost
the same as my fix. Now that you told me that some glyphes can share the
same name and that the nglyphs in the post table is defined as the
number of glyphs, not the number of names, I understand it fully.

> As a result, here is an extract of running testnames against IMPACT:
>   Glyph 188  name fraction -6 40 7 19 41
>   Glyph 189  name Euro -1 40 27 26 41
>   Glyph 190  name guilsinglleft 1 31 10 8 28
> <...>
>   Glyph 246  name threequarters 1 40 35 33 40
>   Glyph 247  name francXXX 2 40 37 34 40
>   Glyph 248  name Gbreve 2 48 28 24 49
>   Glyph 249  name gbreve 2 42 27 23 48
>   Glyph 250  name Idotaccent 2 47 14 10 47
>   Glyph 251  name Scedilla 1 40 26 24 49
>   Glyph 252  name scedilla 1 33 25 23 42
>   Glyph 253  name Cacute 2 47 28 24 48
>   Glyph 254  name cacute 1 41 24 22 42
>   Glyph 255  name Ccaron 2 47 28 24 48
>   Glyph 256  name ccaron 1 41 24 22 42
>   Glyph 257  name dmacron 1 39 27 25 40
>   Glyph 258  name overscore 0 43 28 28 2
>   Glyph 259  name middot 14 19 17 7 6
>   Glyph 260  name foursuperior 0 40 16 15 20
>   Glyph 261  name commaaccent1 6 -3 17 5 10
>   Glyph 262  name Amacron 0 46 26 26 46
>   Glyph 263  name amacron 1 39 26 23 40
>   Glyph 264  name Abreve 0 48 26 26 48
>   Glyph 265  name abreve 1 42 26 23 43
>   Glyph 266  name Aogonek 0 38 26 26 46
>   Glyph 267  name aogonek 1 32 25 23 40
>   Glyph 268  name Ccircumflex 2 47 28 24 48
>   Glyph 269  name ccircumflex 1 41 24 22 42
>   Glyph 270  name Cdot 2 47 28 24 48
>   Glyph 271  name cdot 1 41 24 22 42
> <...>
> 
> Which looks like OK to me (but I wait for aggreement from you).
> 
> So instead of applying your patch, I shall prefer to apply the
> FT2 code instead. See patch included (after the patch from J.-C.)

In fact, I wanted to produce a patch closer to the solution
in FT2 today. I am going to check yours. From a quick review (no
compiling) it looks OK.

> One potential problem of this patch, though, is that I have to
> touch the .h, thus potentially asking for a new revision scheme.
> Werner please comment about the incidence (or lack of).
> 
> The patch is NOT applied to the current CVS tree. I shall commit it
> as soon as Jean-Christophe agrees it works for him.

I think the .h modification can be avoided, at the cost of more CPU
being used. Instead of computing the number of names and storing it,
it can be retrieved at the time of destruction.