Hello,
The attached patch improve my submission to <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED] 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
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 <[EMAIL PROTECTED]> 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.
+ *
+ * <[EMAIL PROTECTED]> 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
+ * <[EMAIL PROTECTED]>*/
+ 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);
