diff --git a/src/plugins/valgrind/callgrind/callgrindcallmodel.cpp b/src/plugins/valgrind/callgrind/callgrindcallmodel.cpp index b341e29d5d1..c08256db2f1 100644 --- a/src/plugins/valgrind/callgrind/callgrindcallmodel.cpp +++ b/src/plugins/valgrind/callgrind/callgrindcallmodel.cpp @@ -5,7 +5,6 @@ #include "callgrindfunctioncall.h" #include "callgrindfunction.h" -#include "callgrindparsedata.h" #include "../valgrindtr.h" #include @@ -17,7 +16,7 @@ namespace Valgrind::Callgrind { class CallModel::Private { public: - const ParseData *m_data = nullptr; + ParseDataPtr m_data = nullptr; QList m_calls; int m_event = 0; const Function *m_function = nullptr; @@ -69,7 +68,7 @@ int CallModel::costEvent() const return d->m_event; } -void CallModel::setParseData(const ParseData *data) +void CallModel::setParseData(const ParseDataPtr &data) { if (d->m_data == data) return; @@ -80,7 +79,7 @@ void CallModel::setParseData(const ParseData *data) d->m_data = data; } -const ParseData *CallModel::parseData() const +ParseDataPtr CallModel::parseData() const { return d->m_data; } diff --git a/src/plugins/valgrind/callgrind/callgrindcallmodel.h b/src/plugins/valgrind/callgrind/callgrindcallmodel.h index 22db3ec1596..2bb0225ace7 100644 --- a/src/plugins/valgrind/callgrind/callgrindcallmodel.h +++ b/src/plugins/valgrind/callgrind/callgrindcallmodel.h @@ -5,6 +5,8 @@ #include "callgrindabstractmodel.h" +#include "callgrindparsedata.h" + #include namespace Valgrind::Callgrind { @@ -29,8 +31,8 @@ public: int costEvent() const; void setCostEvent(int event); - void setParseData(const ParseData *data); - const ParseData *parseData() const; + void setParseData(const ParseDataPtr &data); + ParseDataPtr parseData() const; void setCalls(const QList &calls, const Function *function); QList calls() const; diff --git a/src/plugins/valgrind/callgrind/callgrindcostitem.cpp b/src/plugins/valgrind/callgrind/callgrindcostitem.cpp index d6fa4f99013..8ae0b86190f 100644 --- a/src/plugins/valgrind/callgrind/callgrindcostitem.cpp +++ b/src/plugins/valgrind/callgrind/callgrindcostitem.cpp @@ -3,8 +3,8 @@ #include "callgrindcostitem.h" -#include "callgrindparsedata.h" #include "callgrindfunctioncall.h" +#include "callgrindparsedata.h" #include @@ -13,18 +13,18 @@ namespace Valgrind::Callgrind { class CostItem::Private { public: - Private(ParseData *data); + Private(const ParseData *data); ~Private(); QList m_positions; QList m_events; const FunctionCall *m_call = nullptr; - const ParseData *m_data = nullptr; + const ParseData *m_data; qint64 m_differingFileId = -1; }; -CostItem::Private::Private(ParseData *data) +CostItem::Private::Private(const ParseData *data) : m_positions(data->positions().size(), 0) , m_events(data->events().size(), 0) , m_data(data) @@ -36,9 +36,8 @@ CostItem::Private::~Private() delete m_call; } - //BEGIN CostItem -CostItem::CostItem(ParseData *data) +CostItem::CostItem(const ParseData *data) : d(new Private(data)) { } diff --git a/src/plugins/valgrind/callgrind/callgrindcostitem.h b/src/plugins/valgrind/callgrind/callgrindcostitem.h index 3cc54df1dab..6080d2fd52c 100644 --- a/src/plugins/valgrind/callgrind/callgrindcostitem.h +++ b/src/plugins/valgrind/callgrind/callgrindcostitem.h @@ -18,7 +18,7 @@ class CostItem public: /// @p data the file data this cost item was parsed in. /// required for decompression of string data like differing source file information - explicit CostItem(ParseData *data); + explicit CostItem(const ParseData *data); ~CostItem(); /** diff --git a/src/plugins/valgrind/callgrind/callgrinddatamodel.cpp b/src/plugins/valgrind/callgrind/callgrinddatamodel.cpp index a623a6a8c98..1122b2da28f 100644 --- a/src/plugins/valgrind/callgrind/callgrinddatamodel.cpp +++ b/src/plugins/valgrind/callgrind/callgrinddatamodel.cpp @@ -3,9 +3,8 @@ #include "callgrinddatamodel.h" -#include "callgrindparsedata.h" -#include "callgrindfunction.h" #include "callgrindcostitem.h" +#include "callgrindfunction.h" #include "../valgrindtr.h" #include @@ -21,7 +20,7 @@ class DataModel::Private public: void updateFunctions(); - const ParseData *m_data = nullptr; + ParseDataPtr m_data; int m_event = 0; bool m_verboseToolTips = true; bool m_cycleDetection = false; @@ -51,7 +50,7 @@ DataModel::~DataModel() delete d; } -void DataModel::setParseData(const ParseData *data) +void DataModel::setParseData(const ParseDataPtr &data) { if (d->m_data == data) return; @@ -73,7 +72,7 @@ bool DataModel::verboseToolTipsEnabled() const return d->m_verboseToolTips; } -const ParseData *DataModel::parseData() const +ParseDataPtr DataModel::parseData() const { return d->m_data; } diff --git a/src/plugins/valgrind/callgrind/callgrinddatamodel.h b/src/plugins/valgrind/callgrind/callgrinddatamodel.h index a5233519fab..176ed9a7394 100644 --- a/src/plugins/valgrind/callgrind/callgrinddatamodel.h +++ b/src/plugins/valgrind/callgrind/callgrinddatamodel.h @@ -5,12 +5,13 @@ #include "callgrindabstractmodel.h" +#include "callgrindparsedata.h" + #include namespace Valgrind::Callgrind { class Function; -class ParseData; class DataModel : public QAbstractItemModel { @@ -20,8 +21,8 @@ public: DataModel(); ~DataModel() override; - void setParseData(const ParseData *data); - const ParseData *parseData() const; + void setParseData(const ParseDataPtr &data); + ParseDataPtr parseData() const; void setVerboseToolTipsEnabled(bool enabled); bool verboseToolTipsEnabled() const; diff --git a/src/plugins/valgrind/callgrind/callgrindparsedata.cpp b/src/plugins/valgrind/callgrind/callgrindparsedata.cpp index d58450b9684..231e9dd32ea 100644 --- a/src/plugins/valgrind/callgrind/callgrindparsedata.cpp +++ b/src/plugins/valgrind/callgrind/callgrindparsedata.cpp @@ -116,8 +116,7 @@ void ParseData::Private::cycleDetection() ParseData::ParseData(const QString &fileName) : d(new Private(this, fileName)) -{ -} +{} ParseData::~ParseData() { diff --git a/src/plugins/valgrind/callgrind/callgrindparsedata.h b/src/plugins/valgrind/callgrind/callgrindparsedata.h index e6c7b0b8cb1..7f7e864ef47 100644 --- a/src/plugins/valgrind/callgrind/callgrindparsedata.h +++ b/src/plugins/valgrind/callgrind/callgrindparsedata.h @@ -109,4 +109,6 @@ private: Private *d; }; +using ParseDataPtr = std::shared_ptr; + } // namespace Valgrind::Callgrind diff --git a/src/plugins/valgrind/callgrind/callgrindparser.cpp b/src/plugins/valgrind/callgrind/callgrindparser.cpp index b2d4b166835..764b0104674 100644 --- a/src/plugins/valgrind/callgrind/callgrindparser.cpp +++ b/src/plugins/valgrind/callgrind/callgrindparser.cpp @@ -120,11 +120,6 @@ public: { } - ~Private() - { - delete data; - } - void parse(const FilePath &filePath); void parseHeader(QIODevice *device); @@ -145,7 +140,7 @@ public: int addressValuesCount = 0; int costValuesCount = 0; - ParseData *data = nullptr; + std::shared_ptr data; Function *currentFunction = nullptr; qint64 lastObject = -1; qint64 lastFile = -1; @@ -174,17 +169,12 @@ public: void Parser::Private::parse(const FilePath &filePath) { - // be sure to clean up existing data before re-allocating - // the callee might not have taken the parse data - delete data; - data = nullptr; - const QString path = filePath.path(); // FIXME: Works only accidentally for docker QFile file(path); if (!file.open(QIODevice::ReadOnly)) qWarning() << "Could not open file for parsing:" << filePath.toUserOutput(); - data = new ParseData(path); + data = std::make_shared(path); parseHeader(&file); while (!file.atEnd()) { const QByteArray line = file.readLine(); @@ -426,7 +416,7 @@ void Parser::Private::parseCostItem(const char *begin, const char *end) const char *current = begin; QTC_ASSERT(currentDifferingFile == -1 || currentDifferingFile != currentFunction->fileId(), return); - auto costItem = new CostItem(data); + auto costItem = new CostItem(data.get()); costItem->setDifferingFile(currentDifferingFile); FunctionCall *call = nullptr; if (isParsingFunctionCall) { @@ -530,7 +520,7 @@ void Parser::Private::parseSourceFile(const char *begin, const char *end) void Parser::Private::parseFunction(const char *begin, const char *end) { - currentFunction = new Function(data); + currentFunction = new Function(data.get()); currentFunction->setFile(lastFile); currentFunction->setObject(lastObject); @@ -634,11 +624,9 @@ Parser::~Parser() delete d; } -ParseData *Parser::takeData() +ParseDataPtr Parser::parserData() const { - ParseData *data = d->data; - d->data = nullptr; - return data; + return d->data; } } // namespace Valgrind::Callgrind diff --git a/src/plugins/valgrind/callgrind/callgrindparser.h b/src/plugins/valgrind/callgrind/callgrindparser.h index 6355a5859e8..f6211a9aae0 100644 --- a/src/plugins/valgrind/callgrind/callgrindparser.h +++ b/src/plugins/valgrind/callgrind/callgrindparser.h @@ -3,6 +3,8 @@ #pragma once +#include "callgrindparsedata.h" + #include namespace Utils { class FilePath; } @@ -30,7 +32,7 @@ public: // get and take ownership of the parsing results. If this function is not called the repository // will be destroyed when the parser is destroyed. Subsequent calls return null. - ParseData *takeData(); + ParseDataPtr parserData() const; void parse(const Utils::FilePath &filePath); signals: diff --git a/src/plugins/valgrind/callgrind/callgrindproxymodel.cpp b/src/plugins/valgrind/callgrind/callgrindproxymodel.cpp index d9bad9daefc..4dbbdf23d82 100644 --- a/src/plugins/valgrind/callgrind/callgrindproxymodel.cpp +++ b/src/plugins/valgrind/callgrind/callgrindproxymodel.cpp @@ -119,7 +119,7 @@ bool DataProxyModel::filterAcceptsRow(int source_row, const QModelIndex &source_ // check minimum inclusive costs DataModel *model = dataModel(); QTC_ASSERT(model, return false); // as always: this should never happen - const ParseData *data = model->parseData(); + const ParseDataPtr data = model->parseData(); QTC_ASSERT(data, return false); if (m_minimumInclusiveCostRatio != 0.0) { const quint64 totalCost = data->totalCost(0); diff --git a/src/plugins/valgrind/callgrindengine.cpp b/src/plugins/valgrind/callgrindengine.cpp index 8422bb01769..d0fa47853cb 100644 --- a/src/plugins/valgrind/callgrindengine.cpp +++ b/src/plugins/valgrind/callgrindengine.cpp @@ -117,9 +117,9 @@ void CallgrindToolRunner::setToggleCollectFunction(const QString &toggleCollectF m_argumentForToggleCollect = "--toggle-collect=" + toggleCollectFunction; } -Callgrind::ParseData *CallgrindToolRunner::takeParserData() +Callgrind::ParseDataPtr CallgrindToolRunner::parserData() const { - return m_parser.takeData(); + return m_parser.parserData(); } void CallgrindToolRunner::showStatusMessage(const QString &message) diff --git a/src/plugins/valgrind/callgrindengine.h b/src/plugins/valgrind/callgrindengine.h index c0e5ce2a8b6..f5720d2ba0c 100644 --- a/src/plugins/valgrind/callgrindengine.h +++ b/src/plugins/valgrind/callgrindengine.h @@ -23,7 +23,7 @@ public: void start() override; - Valgrind::Callgrind::ParseData *takeParserData(); + Callgrind::ParseDataPtr parserData() const; /// controller actions void dump() { run(Dump); } diff --git a/src/plugins/valgrind/callgrindtool.cpp b/src/plugins/valgrind/callgrindtool.cpp index 15b74a5c5f4..adbc052e070 100644 --- a/src/plugins/valgrind/callgrindtool.cpp +++ b/src/plugins/valgrind/callgrindtool.cpp @@ -95,10 +95,9 @@ public: void setupRunner(CallgrindToolRunner *runner); - void setParseData(ParseData *data); CostDelegate::CostFormat costFormat() const; - void doClear(bool clearParseData); + void doClear(); void updateEventCombo(); signals: @@ -136,10 +135,11 @@ public: void calleeFunctionSelected(const QModelIndex &index); void callerFunctionSelected(const QModelIndex &index); void visualisationFunctionSelected(const Function *function); - void showParserResults(const ParseData *data); + void showParserResults(const ParseDataPtr &data); void takeParserDataFromRunControl(CallgrindToolRunner *rc); - void takeParserData(ParseData *data); + void setParserData(const ParseDataPtr &data); + void doSetParseData(const ParseDataPtr &data); void engineFinished(); void editorOpened(IEditor *); @@ -385,7 +385,7 @@ CallgrindTool::CallgrindTool(QObject *parent) action->setToolTip(Tr::tr("Discard Data")); connect(action, &QAction::triggered, this, [this](bool) { clearTextMarks(); - doClear(true); + doClear(); }); // navigation @@ -494,10 +494,9 @@ CallgrindTool::~CallgrindTool() delete m_visualization; } -void CallgrindTool::doClear(bool clearParseData) +void CallgrindTool::doClear() { - if (clearParseData) // Crashed when done from destructor. - setParseData(nullptr); + doSetParseData({}); // clear filters if (m_filterProjectCosts) @@ -664,25 +663,20 @@ void CallgrindTool::visualisationFunctionSelected(const Function *function) selectFunction(function); } -void CallgrindTool::setParseData(ParseData *data) +void CallgrindTool::doSetParseData(const ParseDataPtr &data) { // we have new parse data, invalidate filters in the proxy model if (m_visualization) m_visualization->setFunction(nullptr); - // invalidate parse data in the data model - delete m_dataModel.parseData(); + // might happen if the user cancelled the profile run + // callgrind then sometimes produces empty callgrind.out.PID files + const ParseDataPtr newData = data && !data->events().isEmpty() ? data : ParseDataPtr(); - if (data && data->events().isEmpty()) { - // might happen if the user cancelled the profile run - // callgrind then sometimes produces empty callgrind.out.PID files - delete data; - data = nullptr; - } - m_lastFileName = data ? data->fileName() : QString(); - m_dataModel.setParseData(data); - m_calleesModel.setParseData(data); - m_callersModel.setParseData(data); + m_lastFileName = newData ? newData->fileName() : QString(); + m_dataModel.setParseData(newData); + m_calleesModel.setParseData(newData); + m_callersModel.setParseData(newData); if (m_eventCombo) updateEventCombo(); @@ -700,7 +694,7 @@ void CallgrindTool::updateEventCombo() m_eventCombo->clear(); - const ParseData *data = m_dataModel.parseData(); + const ParseDataPtr data = m_dataModel.parseData(); if (!data || data->events().isEmpty()) { m_eventCombo->hide(); return; @@ -749,7 +743,7 @@ void CallgrindTool::setupRunner(CallgrindToolRunner *toolRunner) m_dumpAction->setEnabled(true); m_loadExternalLogFile->setEnabled(false); clearTextMarks(); - doClear(true); + doClear(); } void CallgrindTool::updateRunActions() @@ -783,7 +777,7 @@ void CallgrindTool::engineFinished() m_dumpAction->setEnabled(false); m_loadExternalLogFile->setEnabled(true); - const ParseData *data = m_dataModel.parseData(); + const ParseDataPtr data = m_dataModel.parseData(); if (data) showParserResults(data); else @@ -792,7 +786,7 @@ void CallgrindTool::engineFinished() setBusyCursor(false); } -void CallgrindTool::showParserResults(const ParseData *data) +void CallgrindTool::showParserResults(const ParseDataPtr &data) { QString msg; if (data) { @@ -878,15 +872,15 @@ void CallgrindTool::loadExternalLogFile() Parser parser; parser.parse(filePath); - takeParserData(parser.takeData()); + setParserData(parser.parserData()); } void CallgrindTool::takeParserDataFromRunControl(CallgrindToolRunner *rc) { - takeParserData(rc->takeParserData()); + setParserData(rc->parserData()); } -void CallgrindTool::takeParserData(ParseData *data) +void CallgrindTool::setParserData(const ParseDataPtr &data) { showParserResults(data); @@ -895,9 +889,9 @@ void CallgrindTool::takeParserData(ParseData *data) // clear first clearTextMarks(); - doClear(true); + doClear(); + doSetParseData(data); - setParseData(data); const FilePath kcachegrindExecutable = globalSettings().kcachegrindExecutable(); const FilePath found = kcachegrindExecutable.searchInPath(); const bool kcachegrindExists = found.isExecutableFile(); diff --git a/tests/auto/valgrind/callgrind/callgrindparsertests.cpp b/tests/auto/valgrind/callgrind/callgrindparsertests.cpp index 803189e4a8a..76d5282fb66 100644 --- a/tests/auto/valgrind/callgrind/callgrindparsertests.cpp +++ b/tests/auto/valgrind/callgrind/callgrindparsertests.cpp @@ -80,17 +80,16 @@ void CallgrindParserTests::cleanup() { } -ParseData* parseDataFile(const QString &dataFile) +ParseDataPtr parseDataFile(const QString &dataFile) { Parser p; p.parse(Utils::FilePath::fromString(dataFile)); - - return p.takeData(); + return p.parserData(); } void CallgrindParserTests::testHeaderData() { - QScopedPointer data(parseDataFile(dataFile("simpleFunction.out"))); + const ParseDataPtr data(parseDataFile(dataFile("simpleFunction.out"))); QCOMPARE(data->command(), QLatin1String("ls")); QCOMPARE(data->creator(), QLatin1String("callgrind-3.6.0.SVN-Debian")); @@ -109,7 +108,7 @@ void CallgrindParserTests::testHeaderData() void CallgrindParserTests::testSimpleFunction() { - QScopedPointer data(parseDataFile(dataFile("simpleFunction.out"))); + const ParseDataPtr data(parseDataFile(dataFile("simpleFunction.out"))); QCOMPARE(data->functions().size(), 4); @@ -178,7 +177,7 @@ void CallgrindParserTests::testSimpleFunction() void CallgrindParserTests::testCallee() { - QScopedPointer data(parseDataFile(dataFile("calleeFunctions.out"))); + const ParseDataPtr data(parseDataFile(dataFile("calleeFunctions.out"))); QCOMPARE(data->functions().size(), 3); @@ -253,7 +252,7 @@ void CallgrindParserTests::testCallee() void CallgrindParserTests::testInlinedCalls() { - QScopedPointer data(parseDataFile(dataFile("inlinedFunctions.out"))); + const ParseDataPtr data(parseDataFile(dataFile("inlinedFunctions.out"))); QCOMPARE(data->functions().size(), 3); const Function *main = data->functions().first(); @@ -279,7 +278,7 @@ void CallgrindParserTests::testInlinedCalls() void CallgrindParserTests::testMultiCost() { - QScopedPointer data(parseDataFile(dataFile("multiCost.out"))); + const ParseDataPtr data(parseDataFile(dataFile("multiCost.out"))); QCOMPARE(data->functions().size(), 2); QCOMPARE(data->positions(), QStringList() << "line"); @@ -298,7 +297,7 @@ void CallgrindParserTests::testMultiCost() void CallgrindParserTests::testMultiPos() { - QScopedPointer data(parseDataFile(dataFile("multiPos.out"))); + const ParseDataPtr data(parseDataFile(dataFile("multiPos.out"))); QCOMPARE(data->functions().size(), 2); QCOMPARE(data->positions(), QStringList() << "line" << "memAddr"); @@ -316,7 +315,7 @@ void CallgrindParserTests::testMultiPos() void CallgrindParserTests::testMultiPosAndCost() { - QScopedPointer data(parseDataFile(dataFile("multiCostAndPos.out"))); + const ParseDataPtr data(parseDataFile(dataFile("multiCostAndPos.out"))); QCOMPARE(data->functions().size(), 2); QCOMPARE(data->positions(), QStringList() << "line" << "memAddr"); @@ -345,7 +344,7 @@ const Function *findFunction(const QString &needle, const QVector data(parseDataFile(dataFile("cycle.out"))); + const ParseDataPtr data(parseDataFile(dataFile("cycle.out"))); QCOMPARE(data->functions().size(), 4); const Function *main = data->functions().at(0); @@ -377,7 +376,7 @@ void CallgrindParserTests::testCycle() void CallgrindParserTests::testRecursiveCycle() { - QScopedPointer data(parseDataFile(dataFile("recursiveCycle.out"))); + const ParseDataPtr data(parseDataFile(dataFile("recursiveCycle.out"))); QCOMPARE(data->functions().size(), 5); const Function *main = findFunction(QLatin1String("main"), data->functions()); @@ -418,7 +417,7 @@ void CallgrindParserTests::testRecursiveCycle() void CallgrindParserTests::testRecursion() { - QScopedPointer data(parseDataFile(dataFile("recursion.out"))); + const ParseDataPtr data(parseDataFile(dataFile("recursion.out"))); QCOMPARE(data->functions().size(), 3); QCOMPARE(data->totalCost(0), quint64(35700972));