Hi Scuri, "Anyway, I think we may test for a valid item there, but I don't think that every call to GETITEM should be tested and simply return doing nothing." If documentation says that GETITEM can fail, required is, test return. If GETITEM fail, in some situations, return doing nothing, can be better alternative. Or do default action, if GETITEM fail.
Best regards, Ranier Vilela ________________________________________ De: Antonio Scuri <[email protected]> Enviado: quinta-feira, 22 de fevereiro de 2018 16:55 Para: IUP discussion list. Assunto: Re: [Iup-users] winiup tree bug: Message from an item already deleted Hi, I run the sample here in debug, and with the release version, but the problem did not appeared. I understand you are rebuilding the tree during a double click. This is what you are doing in your program too or just a test? I wouldn't recommend that. Anyway, I think we may test for a valid item there, but I don't think that every call to GETITEM should be tested and simply return doing nothing. Is there a way that I can force the sample in a situation that the problem would be more likely to occur? Which IUP version are you using? Best, Scuri 2018-02-10 22:58 GMT-02:00 Ranier VF <[email protected]<mailto:[email protected]>>: Hi, Clearly documents says TVM_GETITEM message can fail. https://msdn.microsoft.com/en-us/library/windows/desktop/bb773596(v=vs.85).aspx Iupwin_tree.c there are lot calls to TVM_GETITEM message, without check result. Check if this works? --- a\src\win\iupwin_tree.c Sat Feb 10 21:58:52 2018 +++ b\src\win\iupwin_tree.c Sat Feb 10 22:56:21 2018 @@ -199,14 +199,16 @@ if (hPrevItem) { - int kindPrev; + int kindPrev = ITREE_BRANCH; TVITEM tviPrevItem; /* get the KIND attribute of reference node */ tviPrevItem.hItem = hPrevItem; tviPrevItem.mask = TVIF_PARAM|TVIF_CHILDREN; SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&tviPrevItem); - kindPrev = ((winTreeItemData*)tviPrevItem.lParam)->kind; + if (tviPrevItem.lParam) { + kindPrev = ((winTreeItemData*)tviPrevItem.lParam)->kind; + } /* Define the parent and the position to the new node inside the list, using the KIND attribute of reference node */ @@ -264,29 +266,41 @@ static int winTreeIsItemExpanded(Ihandle* ih, HTREEITEM hItem) { TVITEM item; + item.hItem = hItem; item.mask = TVIF_HANDLE | TVIF_STATE; SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item); - return (item.state & TVIS_EXPANDED) != 0; + if (item.lParam) + { + return (item.state & TVIS_EXPANDED) != 0; + } + else + { + return 0; + } } static int winTreeIsBranch(Ihandle* ih, HTREEITEM hItem) { TVITEM item; - winTreeItemData* itemData; + int kind = ITREE_BRANCH; item.mask = TVIF_HANDLE | TVIF_PARAM; item.hItem = hItem; SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item); - itemData = (winTreeItemData*)item.lParam; + if (item.lParam) + { + winTreeItemData* itemData = (winTreeItemData*)item.lParam; - return (itemData->kind == ITREE_BRANCH); + kind = itemData->kind; + } + + return (kind == ITREE_BRANCH); } static void winTreeExpandItem(Ihandle* ih, HTREEITEM hItem, int expand) { TVITEM item; - winTreeItemData* itemData; iupAttribSet(ih, "_IUPTREE_IGNORE_BRANCH_CB", "1"); /* it only works if the branch has children */ @@ -297,15 +311,21 @@ item.hItem = hItem; item.mask = TVIF_HANDLE|TVIF_PARAM; SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item); - itemData = (winTreeItemData*)item.lParam; - - if (expand) - item.iSelectedImage = item.iImage = (itemData->image_expanded!=-1)? itemData->image_expanded: (int)ih->data->def_image_expanded; - else - item.iSelectedImage = item.iImage = (itemData->image!=-1)? itemData->image: (int)ih->data->def_image_collapsed; + if (item.lParam) + { + winTreeItemData* itemData; - item.hItem = hItem; - item.mask = TVIF_HANDLE | TVIF_IMAGE | TVIF_SELECTEDIMAGE; + itemData = (winTreeItemData*)item.lParam; + if (expand) + item.iSelectedImage = item.iImage = (itemData->image_expanded!=-1)? itemData->image_expanded: (int)ih->data->def_image_expanded; + else + item.iSelectedImage = item.iImage = (itemData->image!=-1)? itemData->image: (int)ih->data->def_image_collapsed; + item.mask = TVIF_HANDLE | TVIF_IMAGE | TVIF_SELECTEDIMAGE; + } + else + { + item.mask = TVIF_HANDLE;; + } SendMessage(ih->handle, TVM_SETITEM, 0, (LPARAM)(LPTVITEM)&item); } @@ -493,13 +513,17 @@ HTREEITEM new_hItem; TVITEM item; TVINSERTSTRUCT tvins; - TCHAR title[255]; + TCHAR title[256]; item.hItem = hItem; item.mask = TVIF_HANDLE | TVIF_PARAM | TVIF_IMAGE | TVIF_SELECTEDIMAGE | TVIF_TEXT | TVIF_STATE | TVIF_PARAM; item.pszText = title; - item.cchTextMax = 255; + item.cchTextMax = sizeof(title) - 1; SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item); + if (!item.lParam) + { + return NULL; + } if (is_copy) /* during a copy the itemdata reference is not reused */ { @@ -551,19 +575,23 @@ TVITEM item; winTreeItemData* itemDataDst; int id_new, count, id_src, id_dst; + int old_count; - int old_count = ih->data->node_count; + /* Get DST node attributes */ + item.hItem = hItemDst; + item.mask = TVIF_HANDLE | TVIF_PARAM | TVIF_STATE; + SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item); + if (!item.lParam) + { + return NULL; + } + old_count = ih->data->node_count; id_src = iupTreeFindNodeId(ih, hItemSrc); id_dst = iupTreeFindNodeId(ih, hItemDst); id_new = id_dst+1; /* contains the position for a copy operation */ - /* Get DST node attributes */ - item.hItem = hItemDst; - item.mask = TVIF_HANDLE | TVIF_PARAM | TVIF_STATE; - SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item); itemDataDst = (winTreeItemData*)item.lParam; - if (itemDataDst->kind == ITREE_BRANCH && (item.state & TVIS_EXPANDED)) { /* copy as first child of expanded branch */ @@ -574,8 +602,7 @@ { if (itemDataDst->kind == ITREE_BRANCH) { - int child_count = iupdrvTreeTotalChildCount(ih, hItemDst); - id_new += child_count; + id_new += iupdrvTreeTotalChildCount(ih, hItemDst); } /* copy as next brother of item or collapsed branch */ @@ -633,6 +660,10 @@ item.hItem = hItem; item.mask = TVIF_HANDLE | TVIF_PARAM | TVIF_IMAGE | TVIF_SELECTEDIMAGE | TVIF_STATE; SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item); + if (!item.lParam) + { + continue; + } itemData = (winTreeItemData*)item.lParam; if (itemData->kind == ITREE_BRANCH) Best, Ranier Vilela ________________________________________ De: 云风 Cloud Wu <[email protected]<mailto:[email protected]>> Enviado: sexta-feira, 9 de fevereiro de 2018 14:08 Para: [email protected]<mailto:[email protected]> Assunto: [Iup-users] winiup tree bug: Message from an item already deleted I found a bug can crash the program today. It crash in static int winTreeCallBranchLeafCb(Ihandle* ih, HTREEITEM hItem) @iupwin_tree.c SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item); itemData = (winTreeItemData*)item.lParam; if (itemData->kind == ITREE_BRANCH) SendMessage may return false, and itemData will be NULL . It may caused by delete nodes in `executeleaf_cb` . It seems that windows may push a notify message before an item deleted, and then it recv this message, but the item is gone. I try to make a minimal working example , but it doesn't work every time. ------- local tree = iup.tree { HIDEBUTTONS = "yes", HIDELINES = "yes", } local dlg = iup.dialog { tree, margin = "4x4", size = "HALFxHALF", shrink="yes", title = "Shader Compiler", } local function do_some_systemcall() for i=1,100 do -- c:\\windows\\win.ini should be exist. local f = io.open("c:\\windows\\win.ini","rb") if f then f:close() end end end local function rebuild_tree(flag, id) tree.delnode0 = "CHILDREN" tree.title0 = "foobar" if flag then tree.addbranch0 = "xxx" tree.value = 1 do_some_systemcall() for i=1,3 do tree.addleaf1 = "xxx" tree.image2 = "IMGCOLLAPSED" end do_some_systemcall() else tree.addleaf0 = "Click Me (may crash)" tree.image1 = "IMGCOLLAPSED" for i=0,3 do do_some_systemcall() tree.addleaf0 = "xxx" tree.image1 = "IMGCOLLAPSED" end tree.value = id end function tree:executeleaf_cb(id) rebuild_tree(not flag, id) end function tree:branchclose_cb(id) rebuild_tree(not flag, id) return iup.IGNORE end end dlg:showxy(iup.CENTER,iup.CENTER) rebuild_tree(false) iup.MainLoop() iup.Close() ------- Run this program, and click the last item (named "Click Me") , it may crash on windows 10 64bit （1709/16299.192) . I think we should check the result of `SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item)` to fix it, or it there a better way ? ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Iup-users mailing list [email protected]<mailto:[email protected]> https://lists.sourceforge.net/lists/listinfo/iup-users ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Iup-users mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/iup-users
