Hello,
references:
- Described a bug (big memory leaks) in the i18n code to
[EMAIL PROTECTED] -- 18 June 2002 11:32:11 UTC
- As nobody was interested by my bug report produce a first
fix: A.1141 -- 24 June 2002 21:27:58 UTC ([EMAIL PROTECTED])
- Amelioration of A.1141: A.1150 -- 29 June 2002 07:47:29 UTC
([EMAIL PROTECTED], CC to [EMAIL PROTECTED], i.e., try to get
feedback)
- ask for feedback on A.1150 to [EMAIL PROTECTED] --
18 July 2002 02:54:09 UTC -- Get a polite answer.
Now, since four months I try to contact the X experts about
memory leaks in the i18n code. I get no feedback and my fixes
was not applied. So I try again ...
Attached to this message a patch against the current cvs
(the difference with A.1150 is that lib/X11/lcFile.c has changed
recently in the cvs, so I rewrite this part of the patch).
The patch fixes a big memory leak in lib/X11/omGeneric.c:
at some point an XFontStruct is not freed (destroy_fontdata).
Also a "break" is added in lib/X11/omGeberic.c (parse_fontdata),
as suggested in the code by O. Talyor.
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). There are also others small fixes.
The patch also fixes minor memory leaks in lib/X11/lcFile.c
(_XlcLocaleDirName) that I found when I read the i18n code.
Regards, Olivier
Index: lcFile.c
===================================================================
RCS file: /cvs/xc/lib/X11/lcFile.c,v
retrieving revision 3.27
diff -u -r3.27 lcFile.c
--- lcFile.c 2002/09/04 03:09:48 3.27
+++ lcFile.c 2002/10/16 09:18:28
@@ -443,6 +443,10 @@
xlocaledir (dir, PATH_MAX);
n = _XlcParsePath(dir, args, 256);
for (i = 0; i < n; ++i){
+ if (name != NULL && name != lc_name) {
+ XFree(name);
+ name = NULL;
+ }
if ((2 + (args[i] ? strlen(args[i]) : 0) +
strlen(locale_alias)) < PATH_MAX) {
sprintf (buf, "%s/%s", args[i], locale_alias);
@@ -471,15 +475,26 @@
*p = '\0';
break;
}
+ XFree(target_name);
+ target_name = NULL;
}
}
+
+ if (name != NULL && name != lc_name) {
+ XFree(name);
+ }
+
if (target_name == NULL) {
/* vendor locale name == Xlocale name, no expansion of alias */
target_dir = args[0];
target_name = lc_name;
}
+
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/10/16 09:18:41
@@ -1056,6 +1056,21 @@
*
* 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.
+ * Hopefully this also fix a memory leak: if we do not break here
+ * and 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 +1141,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 +1191,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 +1262,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 +1284,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 +1325,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 +1507,13 @@
font_set = gen->font_set;
font_set_num = gen->font_set_num;
for( ; font_set_num-- ; font_set++) {
+ if(font_set->font) {
+ 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);
