On November 24, 2002 02:06 pm, Derick Rethans wrote:
> On Sun, 24 Nov 2002, Ilia Alshanetsky wrote:
> > iliaa Sat Nov 23 20:15:35 2002 EDT
> >
> > Modified files:
> > /php4/ext/gd/libgd gdft.c
> > Log:
> > Fixed 3 memory leaks.
> > Formatted the code to meet PHP's coding standards, which makes it MUCH
> > easier to understand.
>
> But it also makes back porting patched to the real GD _much_ harder, and
> that's why we didn't change the coding style yet AFAIK.
>
The bundled GD has already undergone a number of major modifications and as we
implement the PHP memory wrappers around memory allocation & handling that
difference will only increase. GD 2.0.4 and later already differ
significantly because the code has been 'cleaned-up' by the gd authors, so I
do not believe this is a big concern. On the plus side by increasing the
readability of the code it would be possible to easily to port a few changes
that do happen. As we speak, I am working on porting gd 2.0.7 code changes to
2.0.4 and I can tell you doing diff + patch does not work @ all, beyond very
small chucks of code.
Ilia
> Derick
>
> > Index: php4/ext/gd/libgd/gdft.c
> > diff -u php4/ext/gd/libgd/gdft.c:1.13 php4/ext/gd/libgd/gdft.c:1.14
> > --- php4/ext/gd/libgd/gdft.c:1.13 Sun Nov 17 14:45:10 2002
> > +++ php4/ext/gd/libgd/gdft.c Sat Nov 23 20:15:34 2002
> > @@ -330,153 +330,147 @@
> > return (strcmp (a->fontlist, b->fontlist) == 0);
> > }
> >
> > -static void *
> > -fontFetch (char **error, void *key)
> > +static void *fontFetch (char **error, void *key)
> > {
> > - font_t *a;
> > - fontkey_t *b = (fontkey_t *) key;
> > - int n;
> > - int font_found = 0;
> > - unsigned short platform, encoding;
> > - char *fontsearchpath, *fontlist;
> > - char *fullname = NULL;
> > - char *name, *path, *dir;
> > - char *strtok_ptr;
> > - FT_Error err;
> > - FT_CharMap found = 0;
> > - FT_CharMap charmap;
> > -
> > - a = (font_t *) gdMalloc (sizeof (font_t));
> > - a->fontlist = gdEstrdup (b->fontlist);
> > - a->library = b->library;
> > -
> > - /*
> > - * Search the pathlist for any of a list of font names.
> > - */
> > - fontsearchpath = getenv ("GDFONTPATH");
> > - if (!fontsearchpath)
> > - fontsearchpath = DEFAULT_FONTPATH;
> > - fontlist = gdEstrdup (a->fontlist);
> > -
> > - /*
> > - * Must use gd_strtok_r else pointer corrupted by strtok in nested
> > loop. - */
> > - for (name = gd_strtok_r (fontlist, LISTSEPARATOR, &strtok_ptr); name;
> > - name = gd_strtok_r (0, LISTSEPARATOR, &strtok_ptr))
> > - {
> > -
> > - /* make a fresh copy each time - strtok corrupts it. */
> > - path = gdEstrdup (fontsearchpath);
> > - /*
> > - * Allocate an oversized buffer that is guaranteed to be
> > - * big enough for all paths to be tested.
> > - */
> > - fullname = gdRealloc (fullname,
> > - strlen (fontsearchpath) + strlen (name) + 6);
> > - /* if name is an absolute filename then test directly */
> > - if (*name == '/' || (name[0] != 0 && name[1] == ':' && (name[2] ==
> > '/' || name[2] == '\\'))) - {
> > - sprintf (fullname, "%s", name);
> > - if (access (fullname, R_OK) == 0)
> > - {
> > - font_found++;
> > - break;
> > - }
> > - }
> > - for (dir = strtok (path, PATHSEPARATOR); dir;
> > - dir = strtok (0, PATHSEPARATOR))
> > - {
> > - sprintf (fullname, "%s/%s.ttf", dir, name);
> > - if (access (fullname, R_OK) == 0)
> > - {
> > - font_found++;
> > - break;
> > - }
> > - sprintf (fullname, "%s/%s.pfa", dir, name);
> > - if (access (fullname, R_OK) == 0)
> > - {
> > - font_found++;
> > - break;
> > - }
> > - sprintf (fullname, "%s/%s.pfb", dir, name);
> > - if (access (fullname, R_OK) == 0)
> > - {
> > - font_found++;
> > - break;
> > - }
> > - }
> > - gdFree (path);
> > - if (font_found)
> > - break;
> > - }
> > - gdFree (fontlist);
> > - if (!font_found)
> > - {
> > - *error = "Could not find/open font";
> > - return NULL;
> > - }
> > -
> > - err = FT_New_Face (*b->library, fullname, 0, &a->face);
> > - if (err)
> > - {
> > - *error = "Could not read font";
> > - return NULL;
> > - }
> > - gdFree (fullname);
> > -
> > -/* FIXME - This mapping stuff is imcomplete - where is the spec? */
> > -
> > - a->have_char_map_unicode = 0;
> > - a->have_char_map_big5 = 0;
> > - a->have_char_map_sjis = 0;
> > - a->have_char_map_apple_roman = 0;
> > - for (n = 0; n < a->face->num_charmaps; n++)
> > - {
> > - charmap = a->face->charmaps[n];
> > - platform = charmap->platform_id;
> > - encoding = charmap->encoding_id;
> > - if ((platform == 3 && encoding == 1) /* Windows Unicode */
> > - || (platform == 3 && encoding == 0) /* Windows Symbol */
> > - || (platform == 2 && encoding == 1) /* ISO Unicode */
> > - || (platform == 0))
> > - { /* Apple Unicode */
> > - a->have_char_map_unicode = 1;
> > - found = charmap;
> > - }
> > - else if (platform == 3 && encoding == 4)
> > - { /* Windows Big5 */
> > - a->have_char_map_big5 = 1;
> > - found = charmap;
> > - }
> > - else if (platform == 3 && encoding == 2)
> > - { /* Windows Sjis */
> > - a->have_char_map_sjis = 1;
> > - found = charmap;
> > - }
> > - else if ((platform == 1 && encoding == 0) /* Apple Roman */
> > - || (platform == 2 && encoding == 0))
> > - { /* ISO ASCII */
> > - a->have_char_map_apple_roman = 1;
> > - found = charmap;
> > - }
> > - }
> > - if (!found)
> > - {
> > - *error = "Unable to find a CharMap that I can handle";
> > - return NULL;
> > - }
> > + font_t *a;
> > + fontkey_t *b = (fontkey_t *) key;
> > + int n;
> > + int font_found = 0;
> > + unsigned short platform, encoding;
> > + char *fontsearchpath, *fontlist;
> > + char *fullname = NULL;
> > + char *name, *path=NULL, *dir;
> > + char *strtok_ptr;
> > + FT_Error err;
> > + FT_CharMap found = 0;
> > + FT_CharMap charmap;
> > +
> > + a = (font_t *) gdPMalloc(sizeof(font_t));
> > + a->fontlist = gdPEstrdup(b->fontlist);
> > + a->library = b->library;
> > +
> > + /*
> > + * Search the pathlist for any of a list of font names.
> > + */
> > + fontsearchpath = getenv ("GDFONTPATH");
> > + if (!fontsearchpath) {
> > + fontsearchpath = DEFAULT_FONTPATH;
> > + }
> > + fontlist = gdEstrdup(a->fontlist);
> > +
> > + /*
> > + * Must use gd_strtok_r else pointer corrupted by strtok in nested
> > loop. + */
> > + for (name = gd_strtok_r (fontlist, LISTSEPARATOR, &strtok_ptr); name;
> > name = gd_strtok_r (0, LISTSEPARATOR, &strtok_ptr)) { + /* make a fresh
> > copy each time - strtok corrupts it. */
> > + path = gdEstrdup (fontsearchpath);
> > +
> > + /*
> > + * Allocate an oversized buffer that is guaranteed to be
> > + * big enough for all paths to be tested.
> > + */
> > + fullname = gdRealloc (fullname, strlen (fontsearchpath) + strlen
> > (name) + 6); +
> > + /* if name is an absolute filename then test directly */
> > + if (*name == '/' || (name[0] != 0 && name[1] == ':' && (name[2] == '/'
> > || name[2] == '\\'))) { + sprintf(fullname, "%s", name);
> > + if (access(fullname, R_OK) == 0) {
> > + font_found++;
> > + break;
> > + }
> > + }
> > + for (dir = strtok (path, PATHSEPARATOR); dir; dir = strtok (0,
> > PATHSEPARATOR)) { + sprintf(fullname, "%s/%s.ttf", dir, name);
> > + if (access (fullname, R_OK) == 0) {
> > + font_found++;
> > + break;
> > + }
> > + sprintf(fullname, "%s/%s.pfa", dir, name);
> > + if (access(fullname, R_OK) == 0) {
> > + font_found++;
> > + break;
> > + }
> > + sprintf (fullname, "%s/%s.pfb", dir, name);
> > + if (access(fullname, R_OK) == 0) {
> > + font_found++;
> > + break;
> > + }
> > + }
> > + gdFree(path);
> > + if (font_found) {
> > + break;
> > + }
> > + }
> > +
> > + if (path) {
> > + gdFree(path);
> > + }
> > +
> > + gdFree(fontlist);
> > +
> > + if (!font_found) {
> > + gdPFree(a->fontlist);
> > + gdPFree(a);
> > + *error = "Could not find/open font";
> > + return NULL;
> > + }
> > +
> > + err = FT_New_Face (*b->library, fullname, 0, &a->face);
> > + if (err) {
> > + gdPFree(a->fontlist);
> > + gdPFree(a);
> > + *error = "Could not read font";
> > + return NULL;
> > + }
> > + gdFree(fullname);
> > +
> > + /* FIXME - This mapping stuff is imcomplete - where is the spec? */
> > +
> > + a->have_char_map_unicode = 0;
> > + a->have_char_map_big5 = 0;
> > + a->have_char_map_sjis = 0;
> > + a->have_char_map_apple_roman = 0;
> > + for (n = 0; n < a->face->num_charmaps; n++) {
> > + charmap = a->face->charmaps[n];
> > + platform = charmap->platform_id;
> > + encoding = charmap->encoding_id;
> > + if ((platform == 3 && encoding == 1) /* Windows Unicode */
> > + || (platform == 3 && encoding == 0) /* Windows Symbol */
> > + || (platform == 2 && encoding == 1) /* ISO Unicode */
> > + || (platform == 0))
> > + { /* Apple Unicode */
> > + a->have_char_map_unicode = 1;
> > + found = charmap;
> > + } else if (platform == 3 && encoding == 4) { /* Windows Big5 */
> > + a->have_char_map_big5 = 1;
> > + found = charmap;
> > + } else if (platform == 3 && encoding == 2) { /* Windows Sjis */
> > + a->have_char_map_sjis = 1;
> > + found = charmap;
> > + } else if ((platform == 1 && encoding == 0) /* Apple Roman */
> > + || (platform == 2 && encoding == 0))
> > + { /* ISO ASCII */
> > + a->have_char_map_apple_roman = 1;
> > + found = charmap;
> > + }
> > + }
> > + if (!found) {
> > + gdPFree(a->fontlist);
> > + gdPFree(a);
> > + *error = "Unable to find a CharMap that I can handle";
> > + return NULL;
> > + }
> >
> > - return (void *) a;
> > + return (void *) a;
> > }
> >
> > -static void
> > -fontRelease (void *element)
> > +static void fontRelease (void *element)
> > {
> > - font_t *a = (font_t *) element;
> > + font_t *a = (font_t *) element;
> >
> > - FT_Done_Face (a->face);
> > - gdFree (a->fontlist);
> > - gdFree ((char *) element);
> > + FT_Done_Face (a->face);
> > + gdPFree(a->fontlist);
> > + gdPFree((char *) element);
> > }
> >
> > /********************************************************************/
> >
> >
> >
> > --
> > PHP CVS Mailing List (http://www.php.net/)
> > To unsubscribe, visit: http://www.php.net/unsub.php
--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
