Re: [PATCH 2/2] comctl32: Send CBEM_DELETEITEM when CB_RESETCONTENT is received
On 7/22/2011 06:59, Jay Yang wrote:
diff --git a/dlls/comctl32/comboex.c b/dlls/comctl32/comboex.c index 3402a71..4888519 100644 --- a/dlls/comctl32/comboex.c +++ b/dlls/comctl32/comboex.c @@ -1557,6 +1557,11 @@ static void COMBOEX_ResetContent (COMBOEX_INFO *infoPtr)
item = infoPtr->items; while (item) { + NMCOMBOBOXEXW nmcit; + memset (&nmcit.ceItem, 0, sizeof(nmcit.ceItem)); + nmcit.ceItem.mask=~0; + COMBOEX_CopyItem (item,&nmcit.ceItem); + COMBOEX_NotifyItem (infoPtr, CBEN_DELETEITEM,&nmcit); next = item->next; COMBOEX_FreeText (item); Free (item); First of all no need for memset probably, and it's common rule I think (well for me at least) to avoid unneeded initialization.
As we have message to delete a single item, it's probably time to think about factoring out item deletion helper (that sends notification too). Actually it seems strange for me that this control uses list to store item data, cause messages are clearly index based. So first thing I'd like to see with that is to change item storage to use DPA array as others controls do. But if you don't have time for that now, I could try to do it myself.
On 07/22/2011 12:19 AM, Nikolay Sivov wrote:
On 7/22/2011 06:59, Jay Yang wrote:
diff --git a/dlls/comctl32/comboex.c b/dlls/comctl32/comboex.c index 3402a71..4888519 100644 --- a/dlls/comctl32/comboex.c +++ b/dlls/comctl32/comboex.c @@ -1557,6 +1557,11 @@ static void COMBOEX_ResetContent (COMBOEX_INFO *infoPtr)
item = infoPtr->items; while (item) { + NMCOMBOBOXEXW nmcit; + memset (&nmcit.ceItem, 0, sizeof(nmcit.ceItem)); + nmcit.ceItem.mask=~0; + COMBOEX_CopyItem (item,&nmcit.ceItem); + COMBOEX_NotifyItem (infoPtr, CBEN_DELETEITEM,&nmcit); next = item->next; COMBOEX_FreeText (item); Free (item); First of all no need for memset probably, and it's common rule I think (well for me at least) to avoid unneeded initialization. At the very least I need to set the pszText field to null because COMBOEX_CopyItem looks at that to determine whether to copy the array. Mostly I was just following the other examples in the file.
As we have message to delete a single item, it's probably time to think about factoring out item deletion helper (that sends notification too).
Actually it seems strange for me that this control uses list to store item data, cause messages are clearly index based. So first thing I'd like to see with that is to change item storage to use DPA array as others controls do. But if you don't have time for that now, I could try to do it myself. Yeah, I'm not exactly focused on controls right now, I'm just trying to fix a memory leak that I found in explorer because it doesn't get the CBEM_DELETEITEM notification, so if you know a more correct way to do this, by all means do it. On the otherhand, I do sort of want to get some kind of fix in sometime soon for this because it causes a memory leak in explorer.
-- Jay Yang
On 7/22/2011 08:26, Jay Yang wrote:
On 07/22/2011 12:19 AM, Nikolay Sivov wrote:
On 7/22/2011 06:59, Jay Yang wrote:
diff --git a/dlls/comctl32/comboex.c b/dlls/comctl32/comboex.c index 3402a71..4888519 100644 --- a/dlls/comctl32/comboex.c +++ b/dlls/comctl32/comboex.c @@ -1557,6 +1557,11 @@ static void COMBOEX_ResetContent (COMBOEX_INFO *infoPtr)
item = infoPtr->items; while (item) { + NMCOMBOBOXEXW nmcit; + memset (&nmcit.ceItem, 0, sizeof(nmcit.ceItem)); + nmcit.ceItem.mask=~0; + COMBOEX_CopyItem (item,&nmcit.ceItem); + COMBOEX_NotifyItem (infoPtr, CBEN_DELETEITEM,&nmcit); next = item->next; COMBOEX_FreeText (item); Free (item); First of all no need for memset probably, and it's common rule I think (well for me at least) to avoid unneeded initialization. At the very least I need to set the pszText field to null because COMBOEX_CopyItem looks at that to determine whether to copy the array. Mostly I was just following the other examples in the file. I see, let it be that way then.
As we have message to delete a single item, it's probably time to think about factoring out item deletion helper (that sends notification too).
Actually it seems strange for me that this control uses list to store item data, cause messages are clearly index based. So first thing I'd like to see with that is to change item storage to use DPA array as others controls do. But if you don't have time for that now, I could try to do it myself. Yeah, I'm not exactly focused on controls right now, I'm just trying to fix a memory leak that I found in explorer because it doesn't get the CBEM_DELETEITEM notification, so if you know a more correct way to do this, by all means do it. On the otherhand, I do sort of want to get some kind of fix in sometime soon for this because it causes a memory leak in explorer. Okay, so I'll take a look at item storage for it in a couple of days.
-- Jay Yang
participants (2)
-
Jay Yang -
Nikolay Sivov