Redundant Crash in CCManager::OnShowCallTip()

[oBFusCATed]

If m_CallTips is empty, we have no chance to update the m_CurCallTip, then m_CurCallTip becomes a wild pointer.

Probably the iterator should be reset by default constructing it before updating the m_CallTips.
The code would look something like myit=MyIterator().
Please check the documentation of std::vector or whatever the type of m_CallTips is.

[ollydbg]

If m_CallTips is empty, we have no chance to update the m_CurCallTip, then m_CurCallTip becomes a wild pointer.

Probably the iterator should be reset by default constructing it before updating the m_CallTips.
The code would look something like myit=MyIterator().
Please check the documentation of std::vector or whatever the type of m_CallTips is.

It looks like there is no way to check whether an iterator is valid, see: c++ - Checking if an iterator is valid - Stack Overflow.

I don't have a good idea to fix our CC issue.

[ollydbg]

One idea to fix this issue:

Code

    wxString curTip;
    // HERE, Check whether m_CurCallTip is invalid(point to the end of m_CallTips)
    if (!m_CallTips.empty() && m_CurCallTip != m_CallTips.end())
        curTip = m_CurCallTip->tip;
    m_CallTips = ccPlugin->GetCallTips(pos, stc->GetStyleAt(pos), ed, argsPos);
    // since m_CallTips get updated, we should update the m_CurCallTip, they are done in
    // the following if/else statement.

    if (!m_CallTips.empty() && (event.GetInt() != FROM_TIMER || argsPos == m_CallTipActive))
    {
        ....
        m_CurCallTip = m_CallTips.begin();
        ....
        // HERE, m_CurCallTip is always valid(points to element of  m_CallTips)
        DoUpdateCallTip(ed);
    }
    else if (m_CallTipActive != wxSCI_INVALID_POSITION)
    {
        static_cast<wxScintilla*>(stc)->CallTipCancel();
        m_CallTipActive = wxSCI_INVALID_POSITION;
        // HERE, make m_CurCallTip invalid
        m_CurCallTip = m_CallTips.end();
    }

See the comments after "// HERE".

EDIT: since only the if clause will call the "DoUpdateCallTip(ed);", the else clause just cancel the calltip.

[ollydbg]

This is the testing patch:

Code

 src/sdk/ccmanager.cpp | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/sdk/ccmanager.cpp b/src/sdk/ccmanager.cpp
index a02c9e6..c3d244c 100644
--- a/src/sdk/ccmanager.cpp
+++ b/src/sdk/ccmanager.cpp
@@ -912,10 +912,15 @@ void CCManager::OnShowCallTip(CodeBlocksEvent& event)
 
     int pos = stc->GetCurrentPos();
     int argsPos = wxSCI_INVALID_POSITION;
+    // save the current tip shown text for later recalling
     wxString curTip;
-    if (!m_CallTips.empty())
+    // check whether m_CurCallTip is invalid(point to the end of m_CallTips)
+    if (!m_CallTips.empty() && m_CurCallTip != m_CallTips.end())
         curTip = m_CurCallTip->tip;
+
     m_CallTips = ccPlugin->GetCallTips(pos, stc->GetStyleAt(pos), ed, argsPos);
+    // since m_CallTips get updated, we should update the m_CurCallTip, they are done in
+    // the following if/else statement.
     if (!m_CallTips.empty() && (event.GetInt() != FROM_TIMER || argsPos == m_CallTipActive))
     {
         int lnStart = stc->PositionFromLine(stc->LineFromPosition(pos));
@@ -983,6 +988,8 @@ void CCManager::OnShowCallTip(CodeBlocksEvent& event)
     {
         static_cast<wxScintilla*>(stc)->CallTipCancel();
         m_CallTipActive = wxSCI_INVALID_POSITION;
+        // make m_CurCallTip invalid
+        m_CurCallTip = m_CallTips.end();
     }
 }
 

With the log message:

Code

* SDK: ccmanager: when m_CallTips get updated, also update the iterator.

This try to fix a crash reported here:
http://forums.codeblocks.org/index.php/topic,20489.0.html
Because if we don't update the iterator(m_CurCallTip), the iterator becomes
a wild pointer.

It is against SVN 10391.
Comments are welcome.

[oBFusCATed]

After looking at the docs for vector/iterator I think that the iterator is not a good choice for this case.
If I were you, I'd replace it with a simple index variable.

[ollydbg]

After looking at the docs for vector/iterator I think that the iterator is not a good choice for this case.
If I were you, I'd replace it with a simple index variable.

Indeed, this idea also comes to my mind, but that need a lot of code changes, I'm not fully understand the ccmanager.h/cpp, so let's do it step by step. Also, I need Alpha's comment.
Also, I want to know whether it fixes the crash issue. Since I don't have this crash in my computer even without this patch.

[ollydbg]

Here, why you use the static_cast<wxScintilla*>(stc)->CallTipCancel();? You want to skip the condition check on m_tabSmartJump? and force to cancel it?

Yes; calling base class to force it to cancel.

OK, thanks, then do we use the wxScintilla::CanTipCancel() in the void CCManager::OnEditorTooltip()?

I think I can answer this question myself, this won't cause some memory leak if the tip window is not cancled, then we try to show it again, the already opened tip window will be reused. When we call stc->CallTipShow(pos, tip);, we will finally reach the blow code:

Code

void ScintillaWX::CreateCallTipWindow(PRectangle) {
    if (! ct.wCallTip.Created() ) {
        ct.wCallTip = new wxSCICallTip(sci, &ct, this);
        ct.wDraw = ct.wCallTip;
    }
}

[oBFusCATed]

Indeed, this idea also comes to my mind, but that need a lot of code changes, I'm not fully understand the ccmanager.h/cpp, so let's do it step by step.

I think the changes are quite trivial and low-risk.