[comctl32] Reduce memory usage of the syslink control (Final attempt)

Thomas Weidenmueller wine-patches at reactsoft.com
Thu Jan 5 15:45:17 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 21:40:35 -0000
@@ -1,7 +1,7 @@
 /*
  * SysLink control
  *
- * Copyright 2004, 2005 Thomas Weidenmueller <w3seek at reactos.com>
+ * Copyright 2004 - 2006 Thomas Weidenmueller <w3seek at reactos.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -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);
@@ -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)
         {
-            di->u.Link.szUrl = Alloc((MAX_LINKID_TEXT + 1) * sizeof(WCHAR));
-            if(!Item->szUrl)
+            lstrcpynW(szUrl, Item->szUrl, nc + 1);
+        }
+        else
+        {
+            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
     {


More information about the wine-patches mailing list