[prev in list] [next in list] [prev in thread] [next in thread] 

List:       xfree-fonts
Subject:    [Fonts]i18n fixes in Xlib, A.1141 improvement
From:       Olivier Chapuis <olivier.chapuis () free ! fr>
Date:       2002-06-29 7:47:29
[Download RAW message or body]

Hello,

The attached patch improve my submission to <fixes@xfree86.org> with
sequence number A.1141 (and subject:)

The new code vs A.1141 add one line of code + some comments which
answer a question of O. Taylor about adding a "break" in the code.
I explain why this "break" should be added (Moreover, this fix a
memory leak and fix slow font loading with a base font name with
a long list of fonts).

I cc this mail to fonts@xfree86.org as I think that some part
of the Xlib i18n code need a real clean up (the patch contains
only fixes). I would like to know if someone maintains or works on
this part of the Xlib code and if I have to start this clean up.
Probably an other mailing list should be use but as I am not
in the xfree86 team (first fix send) I cc to this mailing list.

Here a full change log:

* xc/lib/X11/omGeneric.c (destroy_fontdata):
Free a XFontStruct which should be but was not

* xc/lib/X11/omGeneric.c (parse_vw):
(parse_fontname):
Fixed minor memory leaks

* xc/lib/X11/omGeneric.c (parse_fontname):
break when a match is found

* xc/lib/X11/lcFile.c (_XlcLocaleDirName):
Fixed minor memory leaks

Regards, Olivier

PS: - patch done in xc/lib/X11 with cvs diff -u
- Do not be afraid my C is better than my English
- Please apply the patch it fixes quite dramatic bugs IMHO
["patch.txt" (text/plain)]

Index: lcFile.c
===================================================================
RCS file: /cvs/xc/lib/X11/lcFile.c,v
retrieving revision 3.26
diff -u -r3.26 lcFile.c
--- lcFile.c	2002/05/31 18:45:42	3.26
+++ lcFile.c	2002/06/29 07:55:43
@@ -421,12 +421,18 @@
       sprintf(buf, "%s/locale.dir", target_dir);
       target_name = resolve_name(name, buf, RtoL);
     }
+    if (name != NULL && name != lc_name) {
+	XFree(name);
+	name = NULL;
+    }
     if (target_name != NULL) {
       char *p = 0;
       if ((p = strstr(target_name, "/XLC_LOCALE"))) {
 	*p = '\0';
 	break;
       }
+      XFree(target_name);
+      target_name = NULL;
     }
   }
   if (target_name == NULL) {
@@ -437,5 +443,8 @@
   strcpy(dir_name, target_dir);
   strcat(dir_name, "/");
   strcat(dir_name, target_name);
+  if (target_name != lc_name) {
+      XFree(target_name);
+  }
   return dir_name;
 }
Index: omGeneric.c
===================================================================
RCS file: /cvs/xc/lib/X11/omGeneric.c,v
retrieving revision 3.20
diff -u -r3.20 omGeneric.c
--- omGeneric.c	2001/04/05 17:42:26	3.20
+++ omGeneric.c	2002/06/29 07:55:48
@@ -1056,6 +1056,22 @@
 	     *
 	     * Owen Taylor <otaylor@redhat.com>     12 Jul 2000
 	     */
+	    /* The reason why this routine modifies font_data and has a
+	     * font_data_return is that if it is called with C_PRIMARY, then
+	     * font_data_return is used by the caller and with the others classes
+	     * font_data is used by the caller (font_data can be different
+	     * than font_data_return if we do not break here).
+	     * However, a close look at the code (e.g., the drawing funcs) shows
+	     * that breaking or not here change nothing!! 
+	     * So we should 'break' here and the code needs a clean-up (e.g.,
+	     * some FontStruct are loaded and _never_ used).
+	     * Hopefully this also fix a memory leak: if we do not break here
+	     * a found a match later font_data->xlfd_name is deferenced without
+	     * being freed. Finally, this speed up font loading.
+	     *
+	     * <olivier.chapuis@free.fr>             2002-06-29
+	     */
+	    break;
 	}
 
 	switch(class) {
@@ -1126,13 +1142,21 @@
     int		ret = 0, i = 0;
 
     if(vmap_num > 0) {
-	if(parse_fontdata(oc, font_set, vmap, vmap_num, name_list, count, C_VMAP) == -1)
+	if(parse_fontdata(oc, font_set, vmap, vmap_num, name_list, count,
+			  C_VMAP, &font_data_return) == -1) {
+	    if(font_data_return.xlfd_name != NULL)
+		 XFree(font_data_return.xlfd_name);
 	    return (-1);
+	}
+	if(font_data_return.xlfd_name != NULL)
+	    XFree(font_data_return.xlfd_name);
     }
 
     if(vrotate_num > 0) {
 	ret = parse_fontdata(oc, font_set, (FontData) vrotate, vrotate_num,
 			     name_list, count, C_VROTATE, &font_data_return);
+	if(font_data_return.xlfd_name != NULL)
+	    XFree(font_data_return.xlfd_name);
 	if(ret == -1) {
 	    return (-1);
 	} else if(ret == False) {
@@ -1168,6 +1192,8 @@
 
 	    ret = parse_fontdata(oc, font_set, (FontData) vrotate, vrotate_num,
 				 name_list, count, C_VROTATE, &font_data_return);
+	    if(font_data_return.xlfd_name != NULL)
+		XFree(font_data_return.xlfd_name);
 	    if(ret == -1)
 		return (-1);
 	}
@@ -1237,6 +1263,7 @@
 		font_set->side = font_data_return.side;
 
                 Xfree (font_data_return.xlfd_name);
+                font_data_return.xlfd_name = NULL;
 
 		if(parse_vw(oc, font_set, name_list, count) == -1)
 		    goto err;
@@ -1258,6 +1285,10 @@
 	    ret = parse_fontdata(oc, font_set, font_set->substitute,
 				 font_set->substitute_num,
 				 name_list, count, C_SUBSTITUTE, &font_data_return);
+	    if(font_data_return.xlfd_name != NULL) {
+		XFree(font_data_return.xlfd_name);
+		font_data_return.xlfd_name = NULL;
+	    }
 	    if(ret == -1) {
 		goto err;
 	    } else if(ret == True) {
@@ -1295,6 +1326,8 @@
     XFreeStringList(name_list);
     /* Prevent this from being freed twice */
     oc->core.base_name_list = NULL;
+    if(font_data_return.xlfd_name != NULL)
+	XFree(font_data_return.xlfd_name);
 
     return -1;
 }
@@ -1475,6 +1508,14 @@
 	font_set = gen->font_set;
 	font_set_num = gen->font_set_num;
 	for( ; font_set_num-- ; font_set++) {
+	    if(font_set->font) { /* Added the 2002-06-24 
+				  * <olivier.chapuis@free.fr>*/ 
+		if(font_set->font->fid)
+		    XFreeFont(dpy,font_set->font);
+		else
+		    XFreeFontInfo(NULL, font_set->font, 1);
+		font_set->font = NULL;
+	    }
 	    if(font_set->font_data) {
 		free_fontdataOC(dpy,
 			font_set->font_data, font_set->font_data_count);

_______________________________________________
Fonts mailing list
Fonts@XFree86.Org
http://XFree86.Org/mailman/listinfo/fonts

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic