[comctl32] Reduce memory usage of the syslink control (Take 5)

Thomas Weidenmueller wine-patches at reactsoft.com
Thu Jan 5 11:57:18 CST 2006


Reduce memory usage of the syslink control and fix dependent code.

- Thomas

-- 
P.S.: Please let me know if there's something wrong with this patch or
tell me why it was rejected. Otherwise I'm going to assume the fixes
aren't appreciated or necessary because the implementation is considered
mature and stable.

-------------- next part --------------
Index: dlls/comctl32/syslink.c
===================================================================
RCS file: /home/wine/wine/dlls/comctl32/syslink.c,v
retrieving revision 1.15
diff -u -r1.15 syslink.c
--- dlls/comctl32/syslink.c	21 Nov 2005 13:34:06 -0000	1.15
+++ dlls/comctl32/syslink.c	5 Jan 2006 17:55:09 -0000
@@ -62,7 +62,6 @@
 typedef struct _DOC_ITEM
 {
     struct _DOC_ITEM *Next; /* Address to the next item */
-    LPWSTR Text;            /* Text of the document item */
     UINT nText;             /* Number of characters of the text */
     SL_ITEM_TYPE Type;      /* type of the item */
     PDOC_TEXTBLOCK Blocks;  /* Array of text blocks */
@@ -79,6 +78,7 @@
             UINT Dummy;
         } Text;
     } u;
+    WCHAR Text[1];          /* Text of the document item */
 } DOC_ITEM, *PDOC_ITEM;
 
 typedef struct
@@ -117,8 +117,14 @@
 {
     if(DocItem->Type == slLink)
     {
-        Free(DocItem->u.Link.szID);
-        Free(DocItem->u.Link.szUrl);
+        if (DocItem->u.Link.szID != NULL)
+        {
+            Free(DocItem->u.Link.szID);
+        }
+        if (DocItem->u.Link.szUrl != NULL)
+        {
+            Free(DocItem->u.Link.szUrl);
+        }
     }
 
     /* we don't free Text because it's just a pointer to a character in the
@@ -135,16 +141,16 @@
                                         SL_ITEM_TYPE type, PDOC_ITEM LastItem)
 {
     PDOC_ITEM Item;
-    Item = Alloc(sizeof(DOC_ITEM) + ((textlen + 1) * sizeof(WCHAR)));
+
+    textlen = min(textlen, lstrlenW(Text));
+    Item = Alloc(FIELD_OFFSET(DOC_ITEM, Text[textlen + 1]));
     if(Item == NULL)
     {
         ERR("Failed to alloc DOC_ITEM structure!\n");
         return NULL;
     }
-    textlen = min(textlen, lstrlenW(Text));
 
     Item->Next = NULL;
-    Item->Text = (LPWSTR)(Item + 1);
     Item->nText = textlen;
     Item->Type = type;
     Item->Blocks = NULL;
@@ -159,7 +165,6 @@
     }
     
     lstrcpynW(Item->Text, Text, textlen + 1);
-    Item->Text[textlen] = 0;
     
     return Item;
 }
@@ -351,12 +356,11 @@
                         if(lpID != NULL)
                         {
                             nc = min(lenId, strlenW(lpID));
-                            nc = min(nc, MAX_LINKID_TEXT);
-                            Last->u.Link.szID = Alloc((MAX_LINKID_TEXT + 1) * sizeof(WCHAR));
+                            nc = min(nc, MAX_LINKID_TEXT - 1);
+                            Last->u.Link.szID = Alloc((nc + 1) * sizeof(WCHAR));
                             if(Last->u.Link.szID != NULL)
                             {
                                 lstrcpynW(Last->u.Link.szID, lpID, nc + 1);
-                                Last->u.Link.szID[nc] = 0;
                             }
                         }
                         else
@@ -364,12 +368,11 @@
                         if(lpUrl != NULL)
                         {
                             nc = min(lenUrl, strlenW(lpUrl));
-                            nc = min(nc, L_MAX_URL_LENGTH);
-                            Last->u.Link.szUrl = Alloc((L_MAX_URL_LENGTH + 1) * sizeof(WCHAR));
+                            nc = min(nc, L_MAX_URL_LENGTH - 1);
+                            Last->u.Link.szUrl = Alloc((nc + 1) * sizeof(WCHAR));
                             if(Last->u.Link.szUrl != NULL)
                             {
                                 lstrcpynW(Last->u.Link.szUrl, lpUrl, nc + 1);
-                                Last->u.Link.szUrl[nc] = 0;
                             }
                         }
                         else
@@ -431,12 +434,11 @@
             if(lpID != NULL)
             {
                 nc = min(lenId, strlenW(lpID));
-                nc = min(nc, MAX_LINKID_TEXT);
-                Last->u.Link.szID = Alloc((MAX_LINKID_TEXT + 1) * sizeof(WCHAR));
+                nc = min(nc, MAX_LINKID_TEXT - 1);
+                Last->u.Link.szID = Alloc((nc + 1) * sizeof(WCHAR));
                 if(Last->u.Link.szID != NULL)
                 {
                     lstrcpynW(Last->u.Link.szID, lpID, nc + 1);
-                    Last->u.Link.szID[nc] = 0;
                 }
             }
             else
@@ -444,12 +446,11 @@
             if(lpUrl != NULL)
             {
                 nc = min(lenUrl, strlenW(lpUrl));
-                nc = min(nc, L_MAX_URL_LENGTH);
-                Last->u.Link.szUrl = Alloc((L_MAX_URL_LENGTH + 1) * sizeof(WCHAR));
+                nc = min(nc, L_MAX_URL_LENGTH - 1);
+                Last->u.Link.szUrl = Alloc((nc + 1) * sizeof(WCHAR));
                 if(Last->u.Link.szUrl != NULL)
                 {
                     lstrcpynW(Last->u.Link.szUrl, lpUrl, nc + 1);
-                    Last->u.Link.szUrl[nc] = 0;
                 }
             }
             else
@@ -751,6 +752,7 @@
                             {
                                 Free(bl);
                                 bl = NULL;
+                                nBlocks = 0;
                             }
                             break;
                         }
@@ -769,11 +771,12 @@
                     {
                         Free(bl);
                         bl = NULL;
+                        nBlocks = 0;
                     }
                 }
                 else
                 {
-                    bl = Alloc((nBlocks + 1) * sizeof(DOC_TEXTBLOCK));
+                    bl = Alloc(sizeof(DOC_TEXTBLOCK));
                     if (bl != NULL)
                         nBlocks++;
                 }
@@ -820,8 +823,6 @@
         {
             Current->Blocks = bl;
         }
-        else
-            Current->Blocks = NULL;
     }
     
     SelectObject(hdc, hOldFont);
@@ -878,7 +879,7 @@
                 if((Current->Type == slLink) && (Current->u.Link.state & LIS_FOCUSED) && infoPtr->HasFocus)
                 {
                     COLORREF PrevColor;
-                    PrevColor = SetBkColor(hdc, OldBkColor);
+                    PrevColor = SetTextColor(hdc, OldBkColor);
                     DrawFocusRect(hdc, &bl->rc);
                     SetBkColor(hdc, PrevColor);
                 }
@@ -972,8 +973,7 @@
     /* clear the document */
     SYSLINK_ClearDoc(infoPtr);
     
-    textlen = lstrlenW(Text);
-    if(Text == NULL || textlen == 0)
+    if(Text == NULL || (textlen = lstrlenW(Text)) == 0)
     {
         return TRUE;
     }
@@ -984,8 +984,9 @@
         /* Render text position and word wrapping in memory */
         HDC hdc = GetDC(infoPtr->Self);
         SYSLINK_Render(infoPtr, hdc);
-        SYSLINK_Draw(infoPtr, hdc);
         ReleaseDC(infoPtr->Self, hdc);
+
+        InvalidateRect(infoPtr->Self, NULL, TRUE);
     }
     
     return TRUE;
@@ -1031,8 +1032,10 @@
 static LRESULT SYSLINK_SetItem (SYSLINK_INFO *infoPtr, PLITEM Item)
 {
     PDOC_ITEM di;
+    int nc;
+    PWSTR szId = NULL;
+    PWSTR szUrl = NULL;
     BOOL Repaint = FALSE;
-    BOOL Ret = TRUE;
 
     if(!(Item->mask & LIF_ITEMINDEX) || !(Item->mask & (LIF_FLAGSMASK)))
     {
@@ -1046,52 +1049,71 @@
         ERR("Link %d couldn't be found\n", Item->iLink);
         return FALSE;
     }
-    
-    if(Item->mask & LIF_STATE)
-    {
-        UINT oldstate = di->u.Link.state;
-        /* clear the masked bits */
-        di->u.Link.state &= ~(Item->stateMask & LIS_MASK);
-        /* copy the bits */
-        di->u.Link.state |= (Item->state & Item->stateMask) & LIS_MASK;
-        Repaint = (oldstate != di->u.Link.state);
-        
-        /* update the focus */
-        SYSLINK_SetFocusLink(infoPtr, ((di->u.Link.state & LIS_FOCUSED) ? di : NULL));
-    }
 
     if(Item->mask & LIF_ITEMID)
     {
-        if(!di->u.Link.szID)
+        nc = min(lstrlenW(Item->szID), MAX_LINKID_TEXT - 1);
+        szId = Alloc((nc + 1) * sizeof(WCHAR));
+        if(szId)
         {
-            di->u.Link.szID = Alloc((MAX_LINKID_TEXT + 1) * sizeof(WCHAR));
-            if(!Item->szID)
-            {
-                ERR("Unable to allocate memory for link id\n");
-                Ret = FALSE;
-            }
+            lstrcpynW(szId, Item->szID, nc + 1);
         }
-        if(di->u.Link.szID)
+        else
         {
-            lstrcpynW(di->u.Link.szID, Item->szID, MAX_LINKID_TEXT + 1);
+            ERR("Unable to allocate memory for link id\n");
+            return FALSE;
         }
     }
 
     if(Item->mask & LIF_URL)
     {
-        if(!di->u.Link.szUrl)
+        nc = min(lstrlenW(Item->szUrl), L_MAX_URL_LENGTH - 1);
+        szUrl = Alloc((nc + 1) * sizeof(WCHAR));
+        if(szUrl)
+        {
+            lstrcpynW(szUrl, Item->szUrl, nc + 1);
+        }
+        else
         {
-            di->u.Link.szUrl = Alloc((MAX_LINKID_TEXT + 1) * sizeof(WCHAR));
-            if(!Item->szUrl)
+            if (szId)
             {
-                ERR("Unable to allocate memory for link url\n");
-                Ret = FALSE;
+                Free(szId);
             }
+
+            ERR("Unable to allocate memory for link url\n");
+            return FALSE;
+        }
+    }
+
+    if(Item->mask & LIF_ITEMID)
+    {
+        if(di->u.Link.szID)
+        {
+            Free(di->u.Link.szID);
         }
+        di->u.Link.szID = szId;
+    }
+
+    if(Item->mask & LIF_URL)
+    {
         if(di->u.Link.szUrl)
         {
-            lstrcpynW(di->u.Link.szUrl, Item->szUrl, MAX_LINKID_TEXT + 1);
+            Free(di->u.Link.szUrl);
         }
+        di->u.Link.szUrl = szUrl;
+    }
+
+    if(Item->mask & LIF_STATE)
+    {
+        UINT oldstate = di->u.Link.state;
+        /* clear the masked bits */
+        di->u.Link.state &= ~(Item->stateMask & LIS_MASK);
+        /* copy the bits */
+        di->u.Link.state |= (Item->state & Item->stateMask) & LIS_MASK;
+        Repaint = (oldstate != di->u.Link.state);
+        
+        /* update the focus */
+        SYSLINK_SetFocusLink(infoPtr, ((di->u.Link.state & LIS_FOCUSED) ? di : NULL));
     }
     
     if(Repaint)
@@ -1099,7 +1121,7 @@
         SYSLINK_RepaintLink(infoPtr, di);
     }
     
-    return Ret;
+    return TRUE;
 }
 
 /***********************************************************************
@@ -1137,7 +1159,7 @@
     {
         if(di->u.Link.szID)
         {
-            lstrcpynW(Item->szID, di->u.Link.szID, MAX_LINKID_TEXT + 1);
+            lstrcpyW(Item->szID, di->u.Link.szID);
         }
         else
         {
@@ -1149,7 +1171,7 @@
     {
         if(di->u.Link.szUrl)
         {
-            lstrcpynW(Item->szUrl, di->u.Link.szUrl, L_MAX_URL_LENGTH + 1);
+            lstrcpyW(Item->szUrl, di->u.Link.szUrl);
         }
         else
         {
@@ -1209,7 +1231,7 @@
                 HitTest->item.stateMask = 0;
                 if(Current->u.Link.szID)
                 {
-                    lstrcpynW(HitTest->item.szID, Current->u.Link.szID, MAX_LINKID_TEXT + 1);
+                    lstrcpyW(HitTest->item.szID, Current->u.Link.szID);
                 }
                 else
                 {
@@ -1217,7 +1239,7 @@
                 }
                 if(Current->u.Link.szUrl)
                 {
-                    lstrcpynW(HitTest->item.szUrl, Current->u.Link.szUrl, L_MAX_URL_LENGTH + 1);
+                    lstrcpyW(HitTest->item.szUrl, Current->u.Link.szUrl);
                 }
                 else
                 {
@@ -1279,7 +1301,7 @@
     nml.item.stateMask = 0;
     if(Link->u.Link.szID)
     {
-        lstrcpynW(nml.item.szID, Link->u.Link.szID, MAX_LINKID_TEXT + 1);
+        lstrcpyW(nml.item.szID, Link->u.Link.szID);
     }
     else
     {
@@ -1287,7 +1309,7 @@
     }
     if(Link->u.Link.szUrl)
     {
-        lstrcpynW(nml.item.szUrl, Link->u.Link.szUrl, L_MAX_URL_LENGTH + 1);
+        lstrcpyW(nml.item.szUrl, Link->u.Link.szUrl);
     }
     else
     {
@@ -1531,11 +1553,8 @@
     case WM_SIZE:
     {
         HDC hdc = GetDC(infoPtr->Self);
-        if(hdc != NULL)
-        {
-          SYSLINK_Render(infoPtr, hdc);
-          ReleaseDC(infoPtr->Self, hdc);
-        }
+        SYSLINK_Render(infoPtr, hdc);
+        ReleaseDC(infoPtr->Self, hdc);
         return 0;
     }
 


More information about the wine-patches mailing list