From 92ed6f614fa65f1530305676ab63e78e3819b267 Mon Sep 17 00:00:00 2001 From: germanaizek Date: Fri, 27 May 2022 12:33:26 +0300 Subject: [PATCH] Added nullable checks, fixed minor bugs, delete operators and code refactor --- easy_profiler_converter/converter.cpp | 8 ++-- .../include/easy/serialized_block.h | 8 ++-- easy_profiler_core/nonscoped_block.cpp | 3 ++ easy_profiler_core/reader.cpp | 12 +++--- profiler_gui/arbitrary_value_inspector.cpp | 19 +++++----- profiler_gui/blocks_graphics_view.cpp | 2 +- profiler_gui/dialog.cpp | 2 +- profiler_gui/file_reader.cpp | 5 +-- profiler_gui/graphics_block_item.cpp | 2 +- profiler_gui/graphics_scrollbar.cpp | 4 +- profiler_gui/main_window.cpp | 38 +++++++++++-------- profiler_gui/round_progress_widget.cpp | 2 +- reader/main.cpp | 3 +- sample/main.cpp | 2 +- 14 files changed, 58 insertions(+), 52 deletions(-) diff --git a/easy_profiler_converter/converter.cpp b/easy_profiler_converter/converter.cpp index 5265ef9a..e6ea2679 100644 --- a/easy_profiler_converter/converter.cpp +++ b/easy_profiler_converter/converter.cpp @@ -69,7 +69,7 @@ void JsonExporter::convertChildren(const profiler::reader::BlocksTreeNode& node, convert(child, children.back()); } - json["children"] = children; + json["children"] = std::move(children); } void JsonExporter::convert(const ::std::string& inputFile, const ::std::string& outputFile) const @@ -104,7 +104,7 @@ void JsonExporter::convert(const ::std::string& inputFile, const ::std::string& desc["sourceLine"] = descriptor.lineNumber; } - json["blockDescriptors"] = descriptors; + json["blockDescriptors"] = std::move(descriptors); } // convert threads and blocks @@ -122,7 +122,7 @@ void JsonExporter::convert(const ::std::string& inputFile, const ::std::string& convertChildren(kv.second, thread); } - json["threads"] = threads; + json["threads"] = std::move(threads); } // convert bookmarks @@ -142,7 +142,7 @@ void JsonExporter::convert(const ::std::string& inputFile, const ::std::string& bookmark["text"] = mark.text; } - json["bookmarks"] = bookmarks; + json["bookmarks"] = std::move(bookmarks); } try diff --git a/easy_profiler_core/include/easy/serialized_block.h b/easy_profiler_core/include/easy/serialized_block.h index 6d1a16bc..11267dda 100644 --- a/easy_profiler_core/include/easy/serialized_block.h +++ b/easy_profiler_core/include/easy/serialized_block.h @@ -259,8 +259,8 @@ namespace profiler { using value_type = typename StdType::value_type; const value_type* value() const { return reinterpret_cast(data()); } uint16_t size() const { return m_size / sizeof(value_type); } - value_type operator [] (int i) const { return value()[i]; } - value_type at(int i) const { return value()[i]; } + value_type operator [] (size_t i) const { return value()[i]; } + value_type at(size_t i) const { return value()[i]; } ~Value() = delete; }; @@ -270,8 +270,8 @@ namespace profiler { using value_type = char; const char* value() const { return data(); } uint16_t size() const { return m_size; } - char operator [] (int i) const { return data()[i]; } - char at(int i) const { return data()[i]; } + char operator [] (size_t i) const { return data()[i]; } + char at(size_t i) const { return data()[i]; } const char* c_str() const { return data(); } ~Value() = delete; }; diff --git a/easy_profiler_core/nonscoped_block.cpp b/easy_profiler_core/nonscoped_block.cpp index 6d06aba1..c174c15c 100644 --- a/easy_profiler_core/nonscoped_block.cpp +++ b/easy_profiler_core/nonscoped_block.cpp @@ -71,6 +71,9 @@ void NonscopedBlock::copyname() auto len = strlen(m_name); m_runtimeName = static_cast(malloc(len + 1)); + if (!m_runtimeName) + destroy(); + // memcpy should be faster than strncpy because we know // actual bytes number and both strings have the same size memcpy(m_runtimeName, m_name, len); diff --git a/easy_profiler_core/reader.cpp b/easy_profiler_core/reader.cpp index a15cfe2b..018647fc 100644 --- a/easy_profiler_core/reader.cpp +++ b/easy_profiler_core/reader.cpp @@ -942,7 +942,7 @@ extern "C" PROFILER_API profiler::block_index_t fillTreesFromStream(std::atomic< //validate_pointers(progress, olddata, serialized_descriptors, descriptors, descriptors.size()); uint64_t i = 0; - while (!inStream.eof() && descriptors.size() < descriptors_count) + while (!inStream.eof() && !inStream.fail() && descriptors.size() < descriptors_count) { uint16_t sz = 0; read(inStream, sz); @@ -985,7 +985,7 @@ extern "C" PROFILER_API profiler::block_index_t fillTreesFromStream(std::atomic< ReaderThreadPool pool; - while (!inStream.eof() && threads_read_number++ < header.threads_count) + while (!inStream.eof() && !inStream.fail() && threads_read_number++ < header.threads_count) { EASY_BLOCK("Read thread data", profiler::colors::DarkGreen); @@ -1020,7 +1020,7 @@ extern "C" PROFILER_API profiler::block_index_t fillTreesFromStream(std::atomic< uint32_t blocks_number_in_thread = 0; read(inStream, blocks_number_in_thread); auto threshold = read_number + blocks_number_in_thread; - while (!inStream.eof() && read_number < threshold) + while (!inStream.eof() && !inStream.fail() && read_number < threshold) { EASY_BLOCK("Read context switch", profiler::colors::Green); @@ -1091,7 +1091,7 @@ extern "C" PROFILER_API profiler::block_index_t fillTreesFromStream(std::atomic< blocks_number_in_thread = 0; read(inStream, blocks_number_in_thread); threshold = read_number + blocks_number_in_thread; - while (!inStream.eof() && read_number < threshold) + while (!inStream.eof() && !inStream.fail() && read_number < threshold) { EASY_BLOCK("Read block", profiler::colors::Green); @@ -1282,7 +1282,7 @@ extern "C" PROFILER_API profiler::block_index_t fillTreesFromStream(std::atomic< std::vector stringBuffer; read_number = 0; - while (!inStream.eof() && read_number < header.bookmarks_count) + while (!inStream.eof() && !inStream.fail() && read_number < header.bookmarks_count) { profiler::Bookmark bookmark; @@ -1511,7 +1511,7 @@ extern "C" PROFILER_API bool readDescriptionsFromStream(std::atomic& progre //validate_pointers(progress, olddata, serialized_descriptors, descriptors, descriptors.size()); uint64_t i = 0; - while (!inStream.eof() && descriptors.size() < descriptors_count) + while (!inStream.eof() && !inStream.fail() && descriptors.size() < descriptors_count) { uint16_t sz = 0; read(inStream, sz); diff --git a/profiler_gui/arbitrary_value_inspector.cpp b/profiler_gui/arbitrary_value_inspector.cpp index 1fa3cd34..c4696868 100644 --- a/profiler_gui/arbitrary_value_inspector.cpp +++ b/profiler_gui/arbitrary_value_inspector.cpp @@ -1658,7 +1658,7 @@ GraphicsChart::GraphicsChart(QWidget* _parent) GraphicsChart::~GraphicsChart() { - + delete m_chartItem; } void GraphicsChart::onAutoAdjustChartChanged() @@ -1857,7 +1857,8 @@ void ArbitraryTreeWidgetItem::interrupt() return; m_collection->interrupt(); - m_collection.release(); + auto* raw = m_collection.release(); + delete raw; } profiler::color_t ArbitraryTreeWidgetItem::color() const @@ -2077,10 +2078,10 @@ ArbitraryTreeWidgetItem* findSimilarItem(QTreeWidgetItem* _parentItem, Arbitrary for (int c = 0, childrenCount = _parentItem->childCount(); c < childrenCount; ++c) { auto child = _parentItem->child(c); - if (child->type() == ValueItemType) + if (child != nullptr && child->type() == ValueItemType) { auto item = reinterpret_cast(child); - if (item->getSelfIndexInArray() == index) + if (item != nullptr && item->getSelfIndexInArray() == index) { if (&_item->value() == &item->value()) { @@ -2148,7 +2149,7 @@ ArbitraryValuesWidget::ArbitraryValuesWidget(const QListchildCount(); i < childCount; ++i) { auto child = parentItem->child(i); - if (child->checkState(CheckColumn) != Qt::Checked) + if (child != nullptr && child->checkState(CheckColumn) != Qt::Checked) { newState = Qt::PartiallyChecked; break; @@ -2285,7 +2286,7 @@ void ArbitraryValuesWidget::onItemChanged(QTreeWidgetItem* _item, int _column) for (int i = 0; i < parentItem->childCount(); ++i) { auto child = parentItem->child(i); - if (child->checkState(CheckColumn) != Qt::Checked) + if (child != nullptr && child->checkState(CheckColumn) != Qt::Checked) { newState = Qt::PartiallyChecked; break; @@ -2303,7 +2304,7 @@ void ArbitraryValuesWidget::onItemChanged(QTreeWidgetItem* _item, int _column) for (int i = 0; i < item->childCount(); ++i) { auto child = static_cast(item->child(i)); - if (child->checkState(CheckColumn) != Qt::Checked) + if (child != nullptr && child->checkState(CheckColumn) != Qt::Checked) { child->setCheckState(CheckColumn, Qt::Checked); m_checkedItems.push_back(child); @@ -2343,7 +2344,7 @@ void ArbitraryValuesWidget::onItemChanged(QTreeWidgetItem* _item, int _column) for (int i = 0; i < parentItem->childCount(); ++i) { auto child = parentItem->child(i); - if (child->checkState(CheckColumn) != Qt::Unchecked) + if (child != nullptr && child->checkState(CheckColumn) != Qt::Unchecked) { newState = Qt::PartiallyChecked; break; @@ -2361,7 +2362,7 @@ void ArbitraryValuesWidget::onItemChanged(QTreeWidgetItem* _item, int _column) for (int i = 0; i < item->childCount(); ++i) { auto child = static_cast(item->child(i)); - if (child->checkState(CheckColumn) == Qt::Checked) + if (child != nullptr && child->checkState(CheckColumn) == Qt::Checked) { child->setCheckState(CheckColumn, Qt::Unchecked); uncheckedItems.push_back(child); diff --git a/profiler_gui/blocks_graphics_view.cpp b/profiler_gui/blocks_graphics_view.cpp index 051dc868..1a140444 100644 --- a/profiler_gui/blocks_graphics_view.cpp +++ b/profiler_gui/blocks_graphics_view.cpp @@ -1649,7 +1649,7 @@ void BlocksGraphicsView::mouseReleaseEvent(QMouseEvent* _event) if (changedSelection) { profiler::timestamp_t left=0, right=0; - if (changedSelectionBySelectingItem) + if (selectedBlock != nullptr && changedSelectionBySelectingItem) { left = selectedBlock->tree.node->begin() - m_beginTime; right = selectedBlock->tree.node->end() - m_beginTime; diff --git a/profiler_gui/dialog.cpp b/profiler_gui/dialog.cpp index 48acb716..a4c790df 100644 --- a/profiler_gui/dialog.cpp +++ b/profiler_gui/dialog.cpp @@ -83,9 +83,9 @@ Dialog::Dialog(QWidget* parent, const QString& title, QWidget* content, WindowHe mainLayout->addWidget(m_header, 0, Qt::AlignTop); mainLayout->addWidget(content, 1); - auto buttonsLayout = new QHBoxLayout(m_buttonBox); if (buttons != QMessageBox::NoButton) { + auto buttonsLayout = new QHBoxLayout(m_buttonBox); buttonsLayout->addStretch(1); createButtons(buttonsLayout, buttons); mainLayout->addWidget(m_buttonBox, 0, Qt::AlignBottom); diff --git a/profiler_gui/file_reader.cpp b/profiler_gui/file_reader.cpp index 70f2ff0e..941e23d4 100644 --- a/profiler_gui/file_reader.cpp +++ b/profiler_gui/file_reader.cpp @@ -58,10 +58,7 @@ The Apache License, Version 2.0 (the "License"); #undef max #endif -FileReader::FileReader() -{ - -} +FileReader::FileReader() = default; FileReader::~FileReader() { diff --git a/profiler_gui/graphics_block_item.cpp b/profiler_gui/graphics_block_item.cpp index f8d322ca..14ba2d58 100644 --- a/profiler_gui/graphics_block_item.cpp +++ b/profiler_gui/graphics_block_item.cpp @@ -1267,7 +1267,7 @@ const ::profiler_gui::EasyBlock* GraphicsBlockItem::intersect(const QPointF& _po const auto dw = 5. / currentScale; unsigned int i = 0; size_t itemIndex = ::std::numeric_limits::max(); - size_t firstItem = 0, lastItem = static_cast(level0.size()); + size_t firstItem = 0, lastItem = level0.size(); while (i <= levelIndex) { const auto& level = m_levels[i]; diff --git a/profiler_gui/graphics_scrollbar.cpp b/profiler_gui/graphics_scrollbar.cpp index 7d7871d5..39e4b06e 100644 --- a/profiler_gui/graphics_scrollbar.cpp +++ b/profiler_gui/graphics_scrollbar.cpp @@ -1281,7 +1281,7 @@ void GraphicsHistogramItem::updateImageAsync(QRectF _boundingRect, HistRegime _r if (jt != it) { // bypass merged columns - it = jt; + it = std::move(jt); --it; } } @@ -1496,7 +1496,7 @@ void GraphicsHistogramItem::updateImageAsync(QRectF _boundingRect, HistRegime _r if (jt != it) { // bypass merged columns - it = jt; + it = std::move(jt); --it; } } diff --git a/profiler_gui/main_window.cpp b/profiler_gui/main_window.cpp index f2efe69f..74140e80 100644 --- a/profiler_gui/main_window.cpp +++ b/profiler_gui/main_window.cpp @@ -1002,7 +1002,7 @@ MainWindow::~MainWindow() void MainWindow::validateLastDir() { if (m_lastFiles.empty()) - EASY_GLOBALS.lastFileDir = QString(); + EASY_GLOBALS.lastFileDir.clear(); else EASY_GLOBALS.lastFileDir = QFileInfo(m_lastFiles.front()).absoluteDir().canonicalPath(); } @@ -2061,23 +2061,26 @@ void MainWindow::onListenerTimerTimeout() const auto passedTime = std::chrono::duration_cast(now - m_listenStartTime).count(); const auto seconds = static_cast(passedTime) * 1e-3; - switch (m_listener.regime()) + if (m_listenerDialog) { - case ListenerRegime::Capture: + switch (m_listener.regime()) { - m_listenerDialog->setTitle(QString("Capturing frames... %1s").arg(seconds, 0, 'f', 1)); - break; - } + case ListenerRegime::Capture: + { + m_listenerDialog->setTitle(QString("Capturing frames... %1s").arg(seconds, 0, 'f', 1)); + break; + } - case ListenerRegime::Capture_Receive: - { - m_listenerDialog->setTitle(QString("Receiving data... %1s").arg(seconds, 0, 'f', 1)); - break; - } + case ListenerRegime::Capture_Receive: + { + m_listenerDialog->setTitle(QString("Receiving data... %1s").arg(seconds, 0, 'f', 1)); + break; + } - default: - { - break; + default: + { + break; + } } } @@ -2097,8 +2100,11 @@ void MainWindow::onListenerTimerTimeout() m_listener.finalizeCapture(); - m_listenerDialog->accept(); - m_listenerDialog = nullptr; + if (m_listenerDialog) + { + m_listenerDialog->accept(); + m_listenerDialog = nullptr; + } if (m_listener.size() != 0) { diff --git a/profiler_gui/round_progress_widget.cpp b/profiler_gui/round_progress_widget.cpp index 12c3946d..d2aeec66 100644 --- a/profiler_gui/round_progress_widget.cpp +++ b/profiler_gui/round_progress_widget.cpp @@ -643,7 +643,7 @@ RoundProgressWidget::RoundProgressWidget(const QString& title, QWidget* parent) RoundProgressWidget::~RoundProgressWidget() { - + delete m_indicator; } void RoundProgressWidget::setTitle(const QString& title) diff --git a/reader/main.cpp b/reader/main.cpp index f71f2483..d95d2597 100644 --- a/reader/main.cpp +++ b/reader/main.cpp @@ -23,9 +23,8 @@ class TreePrinter std::vector m_rows; public: - TreePrinter(){ + TreePrinter() = default; - } void addNewRow(int level) { diff --git a/sample/main.cpp b/sample/main.cpp index dafa7e31..e1e0fda8 100644 --- a/sample/main.cpp +++ b/sample/main.cpp @@ -252,7 +252,7 @@ int main(int argc, char* argv[]) EASY_ARRAY("threads count", grrr, 3, false, true, "blabla", profiler::colors::Blue/*, EASY_VIN("threads count")*/, profiler::OFF); #endif - int* intPtr = new int(2); + int* intPtr = new int[2]; EASY_VALUE("count", *intPtr); std::vector threads;