GDB Debugger watches bugs

[straka.milan]

Hello,

I found two GDB Debugger watches related bugs in revision 3515:

1) there are two dialogs for editing watches format -- EditWatchesDlg [Menu->Debug->Edit watches] and EditWatchDlg [accessible
  through debug window Watches, rightclick and "Edit watch"]. On the latter there are only 8 formatting options, the "Floating point" is missing.

It can be fixed by adding line in edit_watches_dlg.xrc, here is svn diff

Code Select Expand

Index: edit_watches_dlg.xrc
===================================================================
--- edit_watches_dlg.xrc        (revision 3515)
+++ edit_watches_dlg.xrc        (working copy)
@@ -201,6 +201,7 @@
                     <item>Hexadecimal</item>
                     <item>Binary</item>
                     <item>Character</item>
+                    <item>Floating point</item>
                   </content>
                   <selection>0</selection>
                   <dimension>2</dimension>


2) quite a "big" bug - when refreshing the tree with watches [debuggertree.cpp, method BuildTree on line 154], you create a new list with new items,
  compare them to the items in the tree, modify the existing [color red when a change is found] and you DELETE the items in the tree that now have no value.
But it is done in a wrong way. To see this, watch an array with 10 values and then set the arraysize to 8 values. Then the array is displayed using singleline mode. But in the tree, 4 children remains! [in next refresh, only 1 and then all chindren are gone]. Let's have a look at the skeleton of the BuildTree method:

Code Select Expand

void DebuggerTree::BuildTree(WatchTreeEntry& entry, wxTreeItemId parent)
{
    ...
    wxTreeItemId item = m_pTree->GetFirstChild(parent, cookie);
    size_t count = 0;
    while (item) {
        // if we still have entries
        if (count < entry.entries.size())
        { ... }
        else {
            // no more entries; delete item
            wxTreeItemId newitem = m_pTree->GetNextChild(parent, cookie);
            m_pTree->Delete(item);
            item = newitem;
        }
    }
   ...
}

The GetNextChild function uses cookie to store "the last child I returned from GetFirst/NextChild call". I suggest it uses the index of last child. But when you do m_pTree->Delete(item) BETWEEN the GetNextChild calls, then the cookie is invalidated -- if they use an index, next GetNextChild call SKIPS one child!
[I hope I made myself clear. The point is, you cannot delete children of a node between GetNextChild calls].

I couldnt find better solution than storing the children in a stl_vector and then deleting them at once. Here is my patch using svn diff, if you would like it:

Code Select Expand

Index: debuggertree.cpp
===================================================================
--- debuggertree.cpp    (revision 3521)
+++ debuggertree.cpp    (working copy)
@@ -167,6 +167,7 @@
#endif
     wxTreeItemId item = m_pTree->GetFirstChild(parent, cookie);
     size_t count = 0;
+    std::vector<wxTreeItemId> to_delete;
     while (item)
     {
         // if we still have entries
@@ -193,10 +194,14 @@
         {
             // no more entries; delete item
             wxTreeItemId newitem = m_pTree->GetNextChild(parent, cookie);
-            m_pTree->Delete(item);
+            to_delete.push_back(item);
             item = newitem;
         }
     }
+    while (to_delete.size()) {
+        m_pTree->Delete(to_delete.back());
+        to_delete.pop_back();
+    }

     // now add any remaining entries
     for (; count < entry.entries.size(); ++count)


Please: when I found a bug, i like to find a solution as well. Should I send these svn diffs along, or does it bothers you?

Have a nice day,
Milan Straka

[killerbot]

thanks for the info, looks great. The best way is to provide a patch on our berlios project page.

I will fix the first part (floating point), the rest is for our debugger guru Yiannis   

[mandrav]

Thanks for the patches. But please try to follow the coding guidelines when writing code for C::B.

[straka.milan]

Sorry,

I am a newbie, I didn't read the rules [:ashamed:]. But i will follow them next time, i promise...
[I didn't mean my code snippets as patches, just small quides for you].

BTW isn't it a problem to use a std::vector in such a clumsy way? But i am begginer in wxWidgets and couldn't find a better way to delete
all remaining child items...

[straka.milan]

It wasn't that bad in the end -- just one { not on a new line

[mandrav]

It wasn't that bad in the end -- just one { not on a new line

Hehe, yes, but how would I "force" you to read the rules if I had told you that much?

[straka.milan]

 :shock: such a trickery :shock:

But it worked well

Just a small question to the "rules" -- should I put the bugs only in berlios bugtracker or should i mention them here?
I am not sure whether the bugtracker on berlios gets enough attention :mrgreen:

[mandrav]

:shock: such a trickery :shock:

But it worked well

Just a small question to the "rules" -- should I put the bugs only in berlios bugtracker or should i mention them here?
I am not sure whether the bugtracker on berlios gets enough attention :mrgreen:

The official place to put your patches would be the patch tracker.