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 <antonio.sc...@gmail.com>
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 
<ranier_...@hotmail.com<mailto:ranier_...@hotmail.com>>:
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 <clou...@gmail.com<mailto:clou...@gmail.com>>
Enviado: sexta-feira, 9 de fevereiro de 2018 14:08
Para: iup-users@lists.sourceforge.net<mailto:iup-users@lists.sourceforge.net>
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
Iup-users@lists.sourceforge.net<mailto:Iup-users@lists.sourceforge.net>
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
Iup-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/iup-users

Reply via email to