[Lesstif] [PATCH?] Memory leak in TextF.c (TextF_InitialiseHighlight)

[Brian Ford]

One of our developers noticed the leak stated above.  He took a stab at a
rough patch, but it probably needs further revision/review.  The patch is
attached.  Here is his commentary:

Noticed a memory leak when calling XmTextFieldSetString.

Traced to XtCalloc in TextF_InitialiseHighlight.

TextF_InitialiseHighlight is called repeatedly and every time allocates
a new highlight list without XtFree of the old one.  initialize also
XtMalloc's a single entry highlight list then calls
TextF_InitialiseHighlight - thereby leaking the initial XtMalloc.

Simple solution is to have TextF_InitialiseHighlight
only XtCalloc once and have initialize NULL out TextF_Highlight(w).list
and call TextF_InitialiseHighlight to do the real work.

Thanks.

-- 
Brian Ford
Senior Realtime Software Engineer
VITAL - Visual Simulation Systems
FlightSafety International
Phone: 314-551-8460
Fax:   314-551-8444

--- /home/ford/downloads/lesstif-0.93.40/lib/Xm/TextF.c 2003-03-04 12:21:24.000000000 
-0600
+++ ./TextF.c   2003-03-10 16:16:54.000000000 -0600
@@ -1128,15 +1128,19 @@ SizeRecalc(XmTextFieldWidget w)
 static void
 TextF_InitialiseHighlight(Widget w)
 {
+    /* This is called repeatedly - avoid memory leak
+    ** reuse original allocation!
+    */
+    if (TextF_Highlight(w).list == NULL)
+    {
        TextF_Highlight(w).list = (_XmHighlightRec *)XtCalloc(8, 
sizeof(_XmHighlightRec));
        TextF_Highlight(w).maximum = 8;
-       TextF_Highlight(w).number = 2;
-       TextF_Highlight(w).list[0].position = 0;
-       TextF_Highlight(w).list[0].mode = XmHIGHLIGHT_NORMAL;
-       TextF_Highlight(w).list[1].position = INT_MAX;
-       TextF_Highlight(w).list[1].mode = XmHIGHLIGHT_NORMAL;
+    }
 
-       TextF_HighlightStart(w) = TextF_HighlightEnd(w) = -1;
+    TextF_Highlight(w).number = 2;
+    TextF_Highlight(w).list[0].position = 0;
+    TextF_Highlight(w).list[0].mode = XmHIGHLIGHT_NORMAL;
+    TextF_Highlight(w).list[1].position = INT_MAX;
+    TextF_Highlight(w).list[1].mode = XmHIGHLIGHT_NORMAL;
+
+    TextF_HighlightStart(w) = TextF_HighlightEnd(w) = -1;
 }
 
 /*
@@ -1317,10 +1321,11 @@ initialize(Widget request, Widget tnew, 
        }
 
        TextF_OldCursorX(w) = -1;

+       /* use TextF_InitialiseHighlight (sp) to do everything here
+       ** instead of later, also avoids memory leak of old XtMalloc
+       ** NOTE: must null out TextF_Highlight(w).list
+       */
-       TextF_HighlightStart(w) = TextF_HighlightEnd(w) = -1;
 
-       TextF__(w).highlight.list=(_XmHighlightRec*)XtMalloc(sizeof(_XmHighlightRec));
-       TextF_HighlightMode(w) = XmHIGHLIGHT_NORMAL;
+
+       TextF_Highlight(w).list = NULL;
+       TextF_InitialiseHighlight(tnew);
+
        TextF_OldHighlightStart(w) = TextF_OldHighlightEnd(w) = -1;
        TextF_XOffset(w) = TextF_OldXOffset(w) = 0;
 
@@ -1355,7 +1360,6 @@ initialize(Widget request, Widget tnew, 
 
        XmDropSiteRegister(tnew, ds_args, n);
 
-       TextF_InitialiseHighlight(tnew);
        w->text.max_char_size = 1;              /* FIX ME */
 }
 
@@ -1433,7 +1437,9 @@ destroy(Widget aw)
 
     XtFree((char *)w->text.extension);
     XtUninstallTranslations(aw);
-    XtFree((char *)TextF__(w).highlight.list);
+
+    /* This is probably not necessary...
+    ** Use the macro... should we null out the old pointer?
+    */
+    XtFree((char *)TextF_Highlight(w).list);
+    TextF_Highlight(w).list = NULL;
     
     /* amai: not sure this is necessary ...
              Check out the register calls for more info.