Developer forums (C::B DEVELOPMENT STRICTLY!) > Development

Redundant Crash in CCManager::OnShowCallTip()

<< < (4/5) > >>

oBFusCATed:

--- Quote from: ollydbg on August 13, 2015, 04:23:57 pm ---If m_CallTips is empty, we have no chance to update the m_CurCallTip, then m_CurCallTip becomes a wild pointer.

--- End quote ---
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:

--- Quote from: oBFusCATed on August 13, 2015, 09:00:26 pm ---
--- Quote from: ollydbg on August 13, 2015, 04:23:57 pm ---If m_CallTips is empty, we have no chance to update the m_CurCallTip, then m_CurCallTip becomes a wild pointer.

--- End quote ---
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.

--- End quote ---
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();
    }

--- End code ---
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();
     }
 }
 


--- End code ---
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.

--- End code ---

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.

Navigation

[0] Message Index

[#] Next page

[*] Previous page