Improve error logging of CDB breakpoint setting.

Output warnings to debugger log. Prevent breakpoints from
being set several times when loading session data by
clearing the complete break handler.
Reviewed-by: hjk <qtc-committer@nokia.com>

5 files changed, 70 insertions, 34 deletions

diff --git a/src/plugins/debugger/breakhandler.cpp b/src/plugins/debugger/breakhandler.cpp
index 1a03743507..b5a94b3348 100644
--- a/src/plugins/debugger/breakhandler.cpp
+++ b/src/plugins/debugger/breakhandler.cpp
@@ -281,8 +281,12 @@ void BreakHandler::removeAt(int index)
 
 void BreakHandler::clear()
 {
-    for (int index = size(); --index >= 0; )
-        removeAt(index);
+    qDeleteAll(m_bp);
+    m_bp.clear();
+    m_enabled.clear();
+    m_disabled.clear();
+    m_removed.clear();
+    m_inserted.clear();
 }
 
 int BreakHandler::findBreakpoint(const BreakpointData &needle)
@@ -328,19 +332,19 @@ void BreakHandler::saveBreakpoints()
         const BreakpointData *data = at(index);
         QMap<QString, QVariant> map;
         if (!data->fileName.isEmpty())
-            map["filename"] = data->fileName;
+            map.insert(QLatin1String("filename"), data->fileName);
         if (!data->lineNumber.isEmpty())
-            map["linenumber"] = data->lineNumber;
+            map.insert(QLatin1String("linenumber"), data->lineNumber);
         if (!data->funcName.isEmpty())
-            map["funcname"] = data->funcName;
+            map.insert(QLatin1String("funcname"), data->funcName);
         if (!data->condition.isEmpty())
-            map["condition"] = data->condition;
+            map.insert(QLatin1String("condition"), data->condition);
         if (!data->ignoreCount.isEmpty())
-            map["ignorecount"] = data->ignoreCount;
+            map.insert(QLatin1String("ignorecount"), data->ignoreCount);
         if (!data->enabled)
-            map["disabled"] = "1";
+            map.insert(QLatin1String("disabled"), QLatin1String("1"));
         if (data->useFullPath)
-            map["usefullpath"] = "1";
+            map.insert(QLatin1String("usefullpath"), QLatin1String("1"));
         list.append(map);
     }
     setSessionValueRequested("Breakpoints", list);
@@ -351,18 +355,31 @@ void BreakHandler::loadBreakpoints()
     QVariant value;
     sessionValueRequested("Breakpoints", &value);
     QList<QVariant> list = value.toList();
-
     clear();
     foreach (const QVariant &var, list) {
         const QMap<QString, QVariant> map = var.toMap();
         BreakpointData *data = new BreakpointData(this);
-        data->fileName = map["filename"].toString();
-        data->lineNumber = map["linenumber"].toString();
-        data->condition = map["condition"].toString();
-        data->ignoreCount = map["ignorecount"].toString();
-        data->funcName = map["funcname"].toString();
-        data->enabled = !map["disabled"].toInt();
-        data->useFullPath = bool(map["usefullpath"].toInt());
+        QVariant v = map.value(QLatin1String("filename"));
+        if (v.isValid())
+            data->fileName = v.toString();
+        v = map.value(QLatin1String("linenumber"));
+        if (v.isValid())
+            data->lineNumber = v.toString();
+        v = map.value(QLatin1String("condition"));
+        if (v.isValid())
+            data->condition = v.toString();
+        v = map.value(QLatin1String("ignorecount"));
+        if (v.isValid())
+            data->ignoreCount = v.toInt();
+        v = map.value(QLatin1String("funcname"));
+        if (v.isValid())
+            data->funcName = v.toString();
+        v = map.value(QLatin1String("disabled"));
+        if (v.isValid())
+            data->enabled = !v.toInt();
+        v = map.value(QLatin1String("usefullpath"));
+        if (v.isValid())
+            data->useFullPath = bool(v.toInt());
         data->markerFileName = data->fileName;
         data->markerLineNumber = data->lineNumber.toInt();
         append(data);
diff --git a/src/plugins/debugger/cdb/cdbbreakpoint.cpp b/src/plugins/debugger/cdb/cdbbreakpoint.cpp
index a785e33feb..ee10e1e70b 100644
--- a/src/plugins/debugger/cdb/cdbbreakpoint.cpp
+++ b/src/plugins/debugger/cdb/cdbbreakpoint.cpp
@@ -407,12 +407,19 @@ static bool setBreakPointEnabledById(CIDebugControl *ctl, unsigned long id, bool
     return true;
 }
 
+static inline QString msgCannotSetBreakAtFunction(const QString &func, const QString &why)
+{
+    return QString::fromLatin1("Cannot set a breakpoint at '%1': %2").arg(func, why);
+}
+
 // Synchronize (halted) engine breakpoints with those of the BreakHandler.
 bool CDBBreakPoint::synchronizeBreakPoints(CIDebugControl* debugControl,
                                            CIDebugSymbols *syms,
                                            BreakHandler *handler,
-                                           QString *errorMessage)
+                                           QString *errorMessage, QStringList *warnings)
 {    
+    errorMessage->clear();
+    warnings->clear();
     // Do an initial check whether we are in a state that allows
     // for modifying  breakPoints
     ULONG engineCount;
@@ -420,25 +427,29 @@ bool CDBBreakPoint::synchronizeBreakPoints(CIDebugControl* debugControl,
         *errorMessage = QString::fromLatin1("Cannot modify breakpoints: %1").arg(*errorMessage);
         return false;
     }
+    QString warning;
     // Insert new ones
     bool updateMarkers = false;
     foreach (BreakpointData *nbd, handler->insertedBreakpoints()) {
+        warning.clear();
         // Function breakpoints: Are the module names specified?
         bool breakPointOk = false;
         if (nbd->funcName.isEmpty()) {
             breakPointOk = true;
         } else {
-            switch (resolveSymbol(syms, &nbd->funcName, errorMessage)) {
+            switch (resolveSymbol(syms, &nbd->funcName, &warning)) {
             case ResolveSymbolOk:
                 breakPointOk = true;
                 break;
             case ResolveSymbolAmbiguous:
-                qWarning("Warning: %s\n", qPrintable(*errorMessage));
+                warnings->push_back(msgCannotSetBreakAtFunction(nbd->funcName, warning));
+                warning.clear();
                 breakPointOk = true;
                 break;
             case ResolveSymbolNotFound:
             case ResolveSymbolError:
-                qWarning("Warning: %s\n", qPrintable(*errorMessage));
+                warnings->push_back(msgCannotSetBreakAtFunction(nbd->funcName, warning));
+                warning.clear();
                 break;
             };
         } // function breakpoint
@@ -447,7 +458,7 @@ bool CDBBreakPoint::synchronizeBreakPoints(CIDebugControl* debugControl,
             quint64 address;
             unsigned long id;
             CDBBreakPoint ncdbbp(*nbd);
-            breakPointOk = ncdbbp.add(debugControl, &address, &id, errorMessage);            
+            breakPointOk = ncdbbp.add(debugControl, &address, &id, &warning);
             if (breakPointOk) {
                 if (debugBP)
                     qDebug() << "Added " << id << " at " << address << ncdbbp;
@@ -464,22 +475,21 @@ bool CDBBreakPoint::synchronizeBreakPoints(CIDebugControl* debugControl,
                 nbd->bpFuncName = nbd->funcName;
             }
         } // had symbol
-        if (!breakPointOk)
-            qWarning("%s\n", qPrintable(*errorMessage));
-    }
+        if (!breakPointOk && !warning.isEmpty())
+            warnings->push_back(warning);    }
     // Delete
     foreach (BreakpointData *rbd, handler->takeRemovedBreakpoints()) {
-        if (!removeBreakPointById(debugControl, rbd->bpNumber.toUInt(), errorMessage))
-            qWarning("%s\n", qPrintable(*errorMessage));
+        if (!removeBreakPointById(debugControl, rbd->bpNumber.toUInt(), &warning))
+            warnings->push_back(warning);
         delete rbd;
     }
     // Enable/Disable
     foreach (BreakpointData *ebd, handler->takeEnabledBreakpoints())
-        if (!setBreakPointEnabledById(debugControl, ebd->bpNumber.toUInt(), true, errorMessage))
-            qWarning("%s\n", qPrintable(*errorMessage));
+        if (!setBreakPointEnabledById(debugControl, ebd->bpNumber.toUInt(), true, &warning))
+            warnings->push_back(warning);
     foreach (BreakpointData *dbd, handler->takeDisabledBreakpoints())
-        if (!setBreakPointEnabledById(debugControl, dbd->bpNumber.toUInt(), false, errorMessage))
-            qWarning("%s\n", qPrintable(*errorMessage));
+        if (!setBreakPointEnabledById(debugControl, dbd->bpNumber.toUInt(), false, &warning))
+            warnings->push_back(warning);
 
     if (updateMarkers)
         handler->updateMarkers();
diff --git a/src/plugins/debugger/cdb/cdbbreakpoint.h b/src/plugins/debugger/cdb/cdbbreakpoint.h
index c739866ff0..7383687a61 100644
--- a/src/plugins/debugger/cdb/cdbbreakpoint.h
+++ b/src/plugins/debugger/cdb/cdbbreakpoint.h
@@ -73,7 +73,8 @@ struct CDBBreakPoint
     static bool getBreakPoints(CIDebugControl* debugControl, QList<CDBBreakPoint> *bps, QString *errorMessage);
     // Synchronize (halted) engine with BreakHandler.
     static bool synchronizeBreakPoints(CIDebugControl* ctl, CIDebugSymbols *syms,
-                                       BreakHandler *bh, QString *errorMessage);
+                                       BreakHandler *bh,
+                                       QString *errorMessage, QStringList *warnings);
 
     // Return a 'canonical' file (using '/' and capitalized drive letter)
     static QString canonicalSourceFile(const QString &f);
diff --git a/src/plugins/debugger/cdb/cdbdebugengine.cpp b/src/plugins/debugger/cdb/cdbdebugengine.cpp
index ccaba94aff..9de6dc9f22 100644
--- a/src/plugins/debugger/cdb/cdbdebugengine.cpp
+++ b/src/plugins/debugger/cdb/cdbdebugengine.cpp
@@ -1268,6 +1268,8 @@ void CdbDebugEngine::selectThread(int index)
 
 void CdbDebugEngine::attemptBreakpointSynchronization()
 {
+    if (!m_d->m_hDebuggeeProcess) // Sometimes called from the breakpoint Window
+        return;
     QString errorMessage;
     if (!m_d->attemptBreakpointSynchronization(&errorMessage))
         warning(msgFunctionFailed(Q_FUNC_INFO, errorMessage));
@@ -1301,10 +1303,15 @@ bool CdbDebugEnginePrivate::attemptBreakpointSynchronization(QString *errorMessa
         return true;
     }
 
-    return CDBBreakPoint::synchronizeBreakPoints(m_cif.debugControl,
+    QStringList warnings;
+    const bool ok = CDBBreakPoint::synchronizeBreakPoints(m_cif.debugControl,
                                                  m_cif.debugSymbols,
                                                  m_debuggerManagerAccess->breakHandler(),
-                                                 errorMessage);
+                                                 errorMessage, &warnings);
+    if (const int warningsCount = warnings.size())        
+        for (int w = 0; w < warningsCount; w++)
+            m_engine->warning(warnings.at(w));
+    return ok;
 }
 
 void CdbDebugEngine::reloadDisassembler()
diff --git a/src/plugins/debugger/cdb/cdbmodules.cpp b/src/plugins/debugger/cdb/cdbmodules.cpp
index a7e2e87754..7385bc27d4 100644
--- a/src/plugins/debugger/cdb/cdbmodules.cpp
+++ b/src/plugins/debugger/cdb/cdbmodules.cpp
@@ -138,6 +138,7 @@ static ResolveSymbolResult resolveSymbol(CIDebugSymbols *syms, QString *symbol,
                                          QStringList *matches,
                                          QString *errorMessage)
 {
+    errorMessage->clear();
     // Is it an incomplete symbol?
     if (symbol->contains(QLatin1Char('!')))
         return ResolveSymbolOk;