[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Devel] Possible bug in ftxpost.c ?
From: |
Antoine Leca |
Subject: |
Re: [Devel] Possible bug in ftxpost.c ? |
Date: |
Mon, 08 Jul 2002 17:30:48 +0200 |
Re-bonjour Jean-Christophe,
En Jean-Christophe Dubacq va escriure:
>
> OK, I now post a patch. I know it's for freetype1, but lots of
> applications still use this (ttf2{tfm,pk} among them, gfontview also...
> in fact, all the font utilities I used).
See below about FT2.
> I worked a bit, and I found a fix. It is a very simple patch to
> ftxpost.c, in lib/extend. (see at end of mail).
I disagree with the patch.
What we did (wrongly) on first instance was to "populate" the
post20->glyphNames array with the additional strings in the same order as
there was a request for an additional name, indicated by index greater than
258. Obviously this is wrong when indexes came out of order (as is the
case in the fix to Impact done for the Euro).
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.
> 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).
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.)
Please test (and please Werner/David/whoever knows, review the
areas marked FIXME, I am unsure about some tests)
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.
> How difficult is it to port an application from freetype1 to freetype2 ?
This question you should better ask in a separate thread, in
address@hidden
> --- ftxpost.c.old Thu Jul 4 16:26:20 2002
> +++ ftxpost.c Thu Jul 4 16:24:39 2002
> @@ -126,7 +126,7 @@
> {
> DEFINE_LOAD_LOCALS( input->stream );
>
> - UShort nameindex, n, num;
> + UShort nameindex, n, k, num;
> Byte len;
>
>
> @@ -177,10 +177,10 @@
>
> /* Now we can read the glyph names which are stored in */
> /* Pascal string format. */
> - for ( n = 0; n < num; n++ )
> + for ( k=0, n = 0; n < num; n++ )
> {
> nameindex = post20->glyphNameIndex[n];
> -
> + /* nameindex is only useful to count the glyphs */
> if ( nameindex < 258 )
> ; /* default Mac glyph, do nothing */
> else
> @@ -192,13 +192,14 @@
>
> FORGET_Frame();
>
> - if ( ALLOC_ARRAY( post20->glyphNames[nameindex - 258],
> + if ( ALLOC_ARRAY( post20->glyphNames[k],
> len + 1, Char ) ||
> - FILE_Read( post20->glyphNames[nameindex - 258], len ) )
> + FILE_Read( post20->glyphNames[k], len ) )
> goto Fail1;
>
> /* we make a C string */
> - post20->glyphNames[nameindex - 258][len] = '\0';
> + post20->glyphNames[k][len] = '\0';
> + k++;
> }
> }
--- ftxpost.h.org Mon Dec 24 00:11:44 2001
+++ ftxpost.h Mon Jul 8 17:12:24 2002
@@ -42,6 +42,7 @@
struct TT_Post_20_
{
TT_UShort numGlyphs;
+ TT_UShort numNames;
TT_UShort* glyphNameIndex;
TT_Char** glyphNames;
};
--- ftxpost.c.org Mon Dec 24 00:12:20 2001
+++ ftxpost.c Mon Jul 8 17:23:04 2002
@@ -126,14 +126,14 @@
{
DEFINE_LOAD_LOCALS( input->stream );
- UShort nameindex, n, num;
+ UShort n, num_glyphs, num_names;
Byte len;
if ( ACCESS_Frame( 2L ) )
return error;
- num = GET_UShort();
+ num_glyphs = GET_UShort();
FORGET_Frame();
@@ -142,71 +142,90 @@
/* There already exist fonts which have more than 32768 glyph names */
/* in this table, so the test for this threshold has been dropped. */
- if ( num > input->numGlyphs )
+
+ if ( num_glyphs > input->numGlyphs )
return TT_Err_Invalid_Post_Table;
- post20->numGlyphs = num;
+ post20->numGlyphs = num_glyphs;
+
+ /* load the indices */
- if ( ALLOC_ARRAY( post20->glyphNameIndex, num, TT_UShort ) )
+ if ( ALLOC_ARRAY( post20->glyphNameIndex, num_glyphs, TT_UShort ) )
return error;
- if ( ACCESS_Frame( num * 2L ) )
+ if ( ACCESS_Frame( num_glyphs * 2L ) )
goto Fail;
- for ( n = 0; n < num; n++ )
+ for ( n = 0; n < num_glyphs; n++ )
{
post20->glyphNameIndex[n] = GET_UShort();
+ }
+
+ FORGET_Frame();
+
+ /* compute number of names stored in table */
+ num_names = 0;
+
+ for ( n = 0; n < num_glyphs; n++ )
+ {
+ UShort idx;
+
- if ( post20->glyphNameIndex[n] > 258 + num )
+ idx = post20->glyphNameIndex[n];
+ if ( idx >= 258 )
{
- FORGET_Frame();
- error = TT_Err_Invalid_Post_Table;
- goto Fail;
+ idx -= 257;
+#if 0 /* FIXME This test has been dropped in Freetype 2. Why? */
+ if ( idx > num_glyphs )
+ {
+ error = TT_Err_Invalid_Post_Table;
+ goto Fail;
+ }
+#endif
+ if ( idx > num_names )
+ num_names = idx;
}
}
- FORGET_Frame();
+ post20->numNames = num_names;
- if ( ALLOC_ARRAY( post20->glyphNames, num, Char* ) )
+ if ( num_names == 0 ) /* nothing more to do */
+ return TT_Err_Ok;
+
+ /* now load the name strings */
+ if ( ALLOC_ARRAY( post20->glyphNames, num_names, Char* ) )
goto Fail;
/* We must initialize the glyphNames array for proper */
/* deallocation. */
- for ( n = 0; n < num; n++ )
+ /* FIXME is it still really needed? */
+ for ( n = 0; n < num_names; n++ )
post20->glyphNames[n] = NULL;
/* Now we can read the glyph names which are stored in */
/* Pascal string format. */
- for ( n = 0; n < num; n++ )
+ for ( n = 0; n < num_names; n++ )
{
- nameindex = post20->glyphNameIndex[n];
-
- if ( nameindex < 258 )
- ; /* default Mac glyph, do nothing */
- else
- {
- if ( ACCESS_Frame( 1L ) )
- goto Fail1;
+ if ( ACCESS_Frame( 1L ) )
+ goto Fail1;
- len = GET_Byte();
+ len = GET_Byte();
- FORGET_Frame();
+ FORGET_Frame();
- if ( ALLOC_ARRAY( post20->glyphNames[nameindex - 258],
- len + 1, Char ) ||
- FILE_Read( post20->glyphNames[nameindex - 258], len ) )
- goto Fail1;
+ if ( ALLOC_ARRAY( post20->glyphNames[n], len + 1, Char ) ||
+ FILE_Read( post20->glyphNames[n], len ) )
+ goto Fail1;
- /* we make a C string */
- post20->glyphNames[nameindex - 258][len] = '\0';
- }
+ /* we make a C string */
+ post20->glyphNames[n][len] = '\0';
}
return TT_Err_Ok;
Fail1:
- for ( n = 0; n < num; n++ )
+ for ( n = 0; n < num_names; n++ )
if ( post20->glyphNames[n] )
FREE( post20->glyphNames[n] );
@@ -311,10 +330,13 @@
break;
case 0x00020000:
- for ( n = 0; n < post->p.post20.numGlyphs; n++ )
- if ( post->p.post20.glyphNames[n] )
- FREE( post->p.post20.glyphNames[n] );
- FREE( post->p.post20.glyphNames );
+ if ( post->p.post20.numNames )
+ {
+ for ( n = 0; n < post->p.post20.numNames; n++ )
+ if ( post->p.post20.glyphNames[n] )
+ FREE( post->p.post20.glyphNames[n] );
+ FREE( post->p.post20.glyphNames );
+ }
FREE( post->p.post20.glyphNameIndex );
break;
[Devel] FreeType2PDF?, Lance Dyas, 2002/07/03
Re: [Devel] Possible bug in ftxpost.c ?, Antoine Leca, 2002/07/08