On Tue 27 Sep 2005 at 21:05:58 +0300, Nadav Har'El wrote:
> I tried another scenario to find leaks. This time, I created many iconic
> windows, in hope of (not) finding leaks in icon handling. The loop I ran
> was:
Here is another patch. It replaces the previous one. I think it should
fix most if not all leaks you reported so far, but notoriously it is
bedtime again :-) I moved some cleanup stuff around but I'm not sure it
is at the best place yet, maybe I'll change it again.
I make these against the current development version, but so far it does
seem to work with the RELEASE version too. If there some fuzz-messages
from patch, you know why.
#
# patch "add_window.c"
# from [79ec0cb30571b095df320d19d752aea845ecafa5]
# to [000f73f70e516dae16feb1c65b9bbf6a32ae81fa]
#
# patch "add_window.h"
# from [6c9c58d6b599571e858da54907abc8fc075acd09]
# to [b3743e7bde54f2ff45467f81288a5a9734d3ca5b]
#
# patch "events.c"
# from [f5776d0db26af573cc1704b9d66115765dd0d72b]
# to [5e5586a270b285c36a7c70163f1044b39108d63c]
#
# patch "icons.c"
# from [8d86ee978eabd18d37f3ed4a6bb1aea3b8958e2d]
# to [a3e4578bb9e0c8cfd01fb9434e10ea18eb63f45d]
#
# patch "icons.h"
# from [34477ed4efc3881059aea57745ad12dfe7864c31]
# to [d6a146e9f7aafb77b5aadd27bcec9705852cea2b]
#
# patch "util.c"
# from [937af1aae5e51088d075a5661574482883d93482]
# to [ffc14c9ce235cc633333e0c011d62918934a0f0e]
#
# patch "util.h"
# from [d30fa13a107095ac626aa8d43f1628beefc283de]
# to [086a3b87e05de5bca8bb5221a7fce8478059865e]
#
--- add_window.c
+++ add_window.c
@@ -207,11 +207,6 @@
unsigned long valuemask; /* mask for create windows */
XSetWindowAttributes attributes; /* attributes for create windows */
int width, height; /* tmp variable */
-#ifdef NO_LOCALE
- Atom actual_type;
- int actual_format;
- unsigned long nitems, bytesafter;
-#endif /* NO_LOCALE */
int ask_user; /* don't know where to put the window */
int gravx, gravy; /* gravity signs for positioning */
int namelen;
@@ -275,11 +270,7 @@
XSelectInput(dpy, tmp_win->w, PropertyChangeMask);
XGetWindowAttributes(dpy, tmp_win->w, &tmp_win->attr);
-#ifndef NO_LOCALE
tmp_win->name = (char*) GetWMPropertyString(tmp_win->w, XA_WM_NAME);
-#else /* NO_LOCALE */
- XFetchName(dpy, tmp_win->w, &tmp_win->name);
-#endif /* NO_LOCALE */
tmp_win->class = NoClass;
XGetClassHint(dpy, tmp_win->w, &tmp_win->class);
FetchWmProtocols (tmp_win);
@@ -373,6 +364,11 @@
if (tmp_win->class.res_class == NULL)
tmp_win->class.res_class = NoName;
+ /*
+ * full_name seems to exist only because in the conditional code below,
+ * name is sometimes changed. In all other cases, name and full_name
+ * seem to be identical. Is that worth it?
+ */
tmp_win->full_name = tmp_win->name;
#ifdef CLAUDE
if (strstr (tmp_win->name, " - Mozilla")) {
@@ -1094,16 +1090,10 @@
tmp_win->title_width = tmp_win->attr.width;
-#ifndef NO_LOCALE
tmp_win->icon_name = (char*) GetWMPropertyString(tmp_win->w,
XA_WM_ICON_NAME);
-#else /* NO_LOCALE */
- if (XGetWindowProperty (dpy, tmp_win->w, XA_WM_ICON_NAME, 0L, 200L, False,
- XA_STRING, &actual_type, &actual_format, &nitems,
- &bytesafter,(unsigned char **)&tmp_win->icon_name))
+ if (!tmp_win->icon_name)
tmp_win->icon_name = tmp_win->name;
-#endif /* NO_LOCALE */
- if (tmp_win->icon_name == NULL) tmp_win->icon_name = tmp_win->name;
#ifdef CLAUDE
if (strstr (tmp_win->icon_name, " - Mozilla")) {
char *moz = strstr (tmp_win->icon_name, " - Mozilla");
@@ -1757,6 +1747,26 @@
Scr->d_visual, valuemask, &attributes);
}
+static void DeleteHighlightWindows(TwmWindow *tmp_win)
+{
+ if (tmp_win->HiliteImage) {
+ if (Scr->HighlightPixmapName) {
+ /*
+ * Image obtained from GetImage(): it is in a cache
+ * so we don't need to free it. There will not be multiple
+ * copies if the same xpm:foo image is requested again.
+ */
+ } else {
+ XFreePixmap (dpy, tmp_win->HiliteImage->pixmap);
+ }
+ free(tmp_win->HiliteImage);
+ }
+ /*
+ * The hilite_wl etc windows don't need to be XDestroyWindow()ed
+ * since that will happen when the parent is destroyed (??)
+ */
+}
+
static void CreateLowlightWindows (TwmWindow *tmp_win)
{
XSetWindowAttributes attributes; /* attributes for create windows */
@@ -2011,7 +2021,45 @@
return;
}
+void DeleteWindowTitlebarButtons(TwmWindow *Tmp_win)
+{
+ if (Tmp_win->title_height) {
+ int nb = Scr->TBInfo.nleft + Scr->TBInfo.nright;
+ XDeleteContext(dpy, Tmp_win->title_w, TwmContext);
+ XDeleteContext(dpy, Tmp_win->title_w, ScreenContext);
+ if (Tmp_win->hilite_wl) {
+ XDeleteContext(dpy, Tmp_win->hilite_wl, TwmContext);
+ XDeleteContext(dpy, Tmp_win->hilite_wl, ScreenContext);
+ }
+ if (Tmp_win->hilite_wr) {
+ XDeleteContext(dpy, Tmp_win->hilite_wr, TwmContext);
+ XDeleteContext(dpy, Tmp_win->hilite_wr, ScreenContext);
+ }
+ if (Tmp_win->lolite_wr) {
+ XDeleteContext(dpy, Tmp_win->lolite_wr, TwmContext);
+ XDeleteContext(dpy, Tmp_win->lolite_wr, ScreenContext);
+ }
+ if (Tmp_win->lolite_wl) {
+ XDeleteContext(dpy, Tmp_win->lolite_wl, TwmContext);
+ XDeleteContext(dpy, Tmp_win->lolite_wl, ScreenContext);
+ }
+ if (Tmp_win->titlebuttons) {
+ int i;
+
+ for (i = 0; i < nb; i++) {
+ XDeleteContext (dpy, Tmp_win->titlebuttons[i].window,
+ TwmContext);
+ XDeleteContext (dpy, Tmp_win->titlebuttons[i].window,
+ ScreenContext);
+ }
+ free(Tmp_win->titlebuttons);
+ }
+ }
+ DeleteHighlightWindows(Tmp_win);
+}
+
+
void SetHighlightPixmap (char *filename)
{
#ifdef VMS
--- add_window.h
+++ add_window.h
@@ -72,6 +72,7 @@
extern void GetGravityOffsets (TwmWindow *tmp, int *xp, int *yp);
extern TwmWindow *AddWindow(Window w, int iconm, IconMgr *iconp);
+extern void DeleteWindowTitlebarButtons(TwmWindow *tmp_win);
extern int MappedNotOverride(Window w);
extern void AddDefaultBindings (void);
extern void GrabButtons(TwmWindow *tmp_win);
--- events.c
+++ events.c
@@ -1343,27 +1343,35 @@
* XXX - are we sure that nobody ever sets these to another constant (check
* twm windows)?
*/
-#define isokay(v) ((v) && (v) != NoName)
if ((tmp->name == tmp->full_name) && (tmp->name == tmp->icon_name)) {
- if (nukefull && nukename && nukeicon && isokay(tmp->name)) XFree
(tmp->name);
+ if (nukefull && nukename && nukeicon)
+ FreeWMPropertyString(tmp->name);
} else
if (tmp->name == tmp->full_name) {
- if (nukename && nukefull && isokay(tmp->name)) XFree (tmp->name);
- if (nukeicon && isokay(tmp->icon_name)) XFree (tmp->icon_name);
+ if (nukename && nukefull)
+ FreeWMPropertyString(tmp->name);
+ if (nukeicon)
+ FreeWMPropertyString(tmp->icon_name);
} else
if (tmp->name == tmp->icon_name) {
- if (nukename && nukeicon && isokay(tmp->name)) XFree (tmp->name);
- if (nukefull && isokay(tmp->full_name)) XFree (tmp->full_name);
+ if (nukename && nukeicon)
+ FreeWMPropertyString(tmp->name);
+ if (nukefull)
+ FreeWMPropertyString(tmp->full_name);
} else
if (tmp->icon_name == tmp->full_name) {
- if (nukeicon && nukefull && isokay(tmp->icon_name)) XFree
(tmp->icon_name);
- if (nukename && isokay(tmp->name)) XFree (tmp->name);
+ if (nukeicon && nukefull)
+ FreeWMPropertyString(tmp->icon_name);
+ if (nukename)
+ FreeWMPropertyString(tmp->name);
} else {
- if (nukefull && isokay(tmp->full_name)) XFree (tmp->full_name);
- if (nukename && isokay(tmp->name)) XFree (tmp->name);
- if (nukeicon && isokay(tmp->icon_name)) XFree (tmp->icon_name);
+ if (nukefull)
+ FreeWMPropertyString(tmp->full_name);
+ if (nukename)
+ FreeWMPropertyString(tmp->name);
+ if (nukeicon)
+ FreeWMPropertyString(tmp->icon_name);
}
-#undef isokay
return;
}
@@ -1465,18 +1473,8 @@
switch (Event.xproperty.atom) {
case XA_WM_NAME:
-#ifndef NO_LOCALE
prop = GetWMPropertyString(Tmp_win->w, XA_WM_NAME);
if (prop == NULL) return;
-#else /* NO_LOCALE */
- if (XGetWindowProperty (dpy, Tmp_win->w, Event.xproperty.atom, 0L,
- MAX_NAME_LEN, False, XA_STRING, &actual,
- &actual_format, &nitems, &bytesafter,
- &prop) != Success ||
- actual == None)
- return;
- if (!prop) prop = NoName;
-#endif /* NO_LOCALE */
#ifdef CLAUDE
if (strstr (prop, " - Mozilla")) {
char *moz = strstr (prop, " - Mozilla");
@@ -1550,18 +1548,8 @@
break;
case XA_WM_ICON_NAME:
-#ifndef NO_LOCALE
prop = GetWMPropertyString(Tmp_win->w, XA_WM_ICON_NAME);
if (prop == NULL) return;
-#else /* NO_LOCALE */
- if (XGetWindowProperty (dpy, Tmp_win->w, Event.xproperty.atom, 0,
- MAX_ICON_NAME_LEN, False, XA_STRING, &actual,
- &actual_format, &nitems, &bytesafter,
- &prop) != Success ||
- actual == None)
- return;
- if (!prop) prop = NoName;
-#endif /* NO_LOCALE */
#ifdef CLAUDE
if (strstr (prop, " - Mozilla")) {
char *moz = strstr (prop, " - Mozilla");
@@ -2246,41 +2234,6 @@
XDeleteContext(dpy, Tmp_win->icon->w, TwmContext);
XDeleteContext(dpy, Tmp_win->icon->w, ScreenContext);
}
- if (Tmp_win->title_height)
- {
- int nb = Scr->TBInfo.nleft + Scr->TBInfo.nright;
- XDeleteContext(dpy, Tmp_win->title_w, TwmContext);
- XDeleteContext(dpy, Tmp_win->title_w, ScreenContext);
- if (Tmp_win->hilite_wl)
- {
- XDeleteContext(dpy, Tmp_win->hilite_wl, TwmContext);
- XDeleteContext(dpy, Tmp_win->hilite_wl, ScreenContext);
- }
- if (Tmp_win->hilite_wr)
- {
- XDeleteContext(dpy, Tmp_win->hilite_wr, TwmContext);
- XDeleteContext(dpy, Tmp_win->hilite_wr, ScreenContext);
- }
- if (Tmp_win->lolite_wr)
- {
- XDeleteContext(dpy, Tmp_win->lolite_wr, TwmContext);
- XDeleteContext(dpy, Tmp_win->lolite_wr, ScreenContext);
- }
- if (Tmp_win->lolite_wl)
- {
- XDeleteContext(dpy, Tmp_win->lolite_wl, TwmContext);
- XDeleteContext(dpy, Tmp_win->lolite_wl, ScreenContext);
- }
- if (Tmp_win->titlebuttons) {
- for (i = 0; i < nb; i++) {
- XDeleteContext (dpy, Tmp_win->titlebuttons[i].window,
- TwmContext);
- XDeleteContext (dpy, Tmp_win->titlebuttons[i].window,
- ScreenContext);
- }
- }
- }
-
if (Scr->cmapInfo.cmaps == &Tmp_win->cmaps)
InstallWindowColormaps(DestroyNotify, &Scr->TwmRoot);
@@ -2298,14 +2251,28 @@
* 9. cwins
* 10. titlebuttons
* 11. window ring
+ * 12. (reserved for squeeze_info)
+ * 13. HiliteImage
+ * 14. iconslist
*/
+
+ DeleteWindowTitlebarButtons(Tmp_win); /* 10,13 */
WMapDestroyWindow (Tmp_win);
if (Tmp_win->gray) XFreePixmap (dpy, Tmp_win->gray);
+ /*
+ * The following should destroy all child windows of the frame too,
+ * which is most of the windows we're concerned with, so anything
+ * related to them must be done before here.
+ * Icons are not child windows.
+ */
XDestroyWindow(dpy, Tmp_win->frame);
- if (Tmp_win->icon && Tmp_win->icon->w && !Tmp_win->icon_not_ours) {
- XDestroyWindow(dpy, Tmp_win->icon->w);
- IconDown (Tmp_win);
+ if (Tmp_win->icon) {
+ if (Tmp_win->icon->w && !Tmp_win->icon_not_ours) {
+ XDestroyWindow(dpy, Tmp_win->icon->w);
+ IconDown (Tmp_win);
+ }
+ free (Tmp_win->icon);
}
Tmp_win->occupation = 0;
RemoveIconManager(Tmp_win); /* 7 */
@@ -2323,10 +2290,9 @@
XFree ((char *)Tmp_win->class.res_name);
if (Tmp_win->class.res_class && Tmp_win->class.res_class != NoName) /* 6 */
XFree ((char *)Tmp_win->class.res_class);
- free_cwins (Tmp_win); /* 9 */
- if (Tmp_win->titlebuttons) /* 10 */
- free ((char *) Tmp_win->titlebuttons);
+ free_cwins (Tmp_win); /* 9 */
remove_window_from_ring (Tmp_win); /* 11 */
+ DeleteIconsList (Tmp_win);
free((char *)Tmp_win);
--- icons.c
+++ icons.c
@@ -752,7 +752,8 @@
}
}
- if (icon->match != match_none) AddToList (&tmp_win->iconslist,
icon->pattern, (char*) icon);
+ if (icon->match != match_none)
+ AddToList (&tmp_win->iconslist, icon->pattern, (char*) icon);
tmp_win->icon = icon;
/* I need to figure out where to put the icon window now, because
@@ -796,6 +797,15 @@
return (0);
}
+void DeleteIconsList(TwmWindow *tmp_win)
+{
+ /*
+ * Only the list itself needs to be freed, since the pointers it
+ * contains point into various lists that belong to Scr.
+ */
+ FreeList(&tmp_win->iconslist);
+}
+
void ShrinkIconTitle (TwmWindow *tmp_win)
{
Icon *icon;
--- icons.h
+++ icons.h
@@ -108,6 +108,7 @@
int stepx, int stepy,
char *ijust, char *just, char *align);
extern int CreateIconWindow(TwmWindow *tmp_win, int def_x, int def_y);
+extern void DeleteIconsList(TwmWindow *tmp_win);
extern void ShrinkIconTitle (TwmWindow *tmp_win);
extern void ExpandIconTitle (TwmWindow *tmp_win);
extern void ReshapeIcon (Icon *icon);
--- util.c
+++ util.c
@@ -3145,7 +3145,6 @@
{
name_list **list;
char fullname [256];
- int startn;
Image *image;
if (name == NULL) return (None);
@@ -3154,9 +3153,10 @@
list = &Scr->ImageCache;
#ifdef XPM
if ((name [0] == '@') || (strncmp (name, "xpm:", 4) == 0)) {
- startn = (name [0] == '@') ? 1 : 4;
- if ((image = (Image*) LookInNameList (*list, name)) == None) {
- sprintf (fullname, "%s%dx%d", name, (int) cp.fore, (int) cp.back);
+ sprintf (fullname, "%s%dx%d", name, (int) cp.fore, (int) cp.back);
+
+ if ((image = (Image*) LookInNameList (*list, fullname)) == None) {
+ int startn = (name [0] == '@') ? 1 : 4;
if ((image = GetXpmImage (name + startn, cp)) != None) {
AddToList (list, fullname, (char*) image);
}
@@ -3186,7 +3186,7 @@
#endif
#if !defined(VMS) || defined(HAVE_XWDFILE_H)
if ((strncmp (name, "xwd:", 4) == 0) || (name [0] == '|')) {
- startn = (name [0] == '|') ? 0 : 4;
+ int startn = (name [0] == '|') ? 0 : 4;
if ((image = (Image*) LookInNameList (*list, name)) == None) {
if ((image = GetXwdImage (&name [startn], cp)) != None) {
AddToList (list, name, (char*) image);
@@ -3885,7 +3885,6 @@
}
}
-#ifndef NO_LOCALE
/***********************************************************************
*
* Procedure:
@@ -3904,6 +3903,19 @@
unsigned char *GetWMPropertyString(Window w, Atom prop)
{
+#ifdef NO_LOCALE
+ Atom actual_type;
+ int actual_format;
+ unsigned long nitems, bytesafter;
+ unsigned char *string;
+
+ if (XGetWindowProperty (dpy, w, prop, 0L, 200L, False,
+ XA_STRING, &actual_type, &actual_format, &nitems,
+ &bytesafter, &string) != Success)
+ string = NULL;
+
+ return string;
+#else
XTextProperty text_prop;
char **text_list;
int text_list_count;
@@ -3970,10 +3982,20 @@
}
return stringptr;
+#endif /* NO_LOCALE */
}
+
+void FreeWMPropertyString(unsigned char *prop)
+{
+ if (prop && (char *)prop != NoName) {
+#ifdef NO_LOCALE
+ XFree(prop);
+#else
+ free(prop);
#endif /* NO_LOCALE */
+ }
+}
-
static void ConstrainLeftTop (int *value, int border)
{
if (*value < border) {
--- util.h
+++ util.h
@@ -140,9 +140,8 @@
extern Image *GetImage (char *name, ColorPair cp);
-#ifndef NO_LOCALE
extern unsigned char *GetWMPropertyString(Window w, Atom prop);
-#endif /* NO_LOCALE */
+extern void FreeWMPropertyString(unsigned char *prop);
extern void ConstrainByBorders1 (int *left, int width, int *top, int height);
extern void ConstrainByBorders (TwmWindow *twmwin,
int *left, int width,
-Olaf.
--
___ Olaf 'Rhialto' Seibert -- You author it, and I'll reader it.
\X/ rhialto/at/xs4all.nl -- Cetero censeo "authored" delendum esse.
