#0: various fixes for windows build (#84)

* #0: various fixes for windows build

* remove warnings that definition of a variable hides previously defined ones
* remove warnings that function argument is not used
* remove warnings that statements are not reached
* event_trace_win: fix compilation with UNICODE

* #0 various windows fixes: react to PR comments

* localize unicode handling of process info to ProcessInfo
* fix compilation error in Properties constructor

* event_trace_win: fix bug in ProcessInfo initialization

We didn't retry setting the name when GetModuleBaseName() didn't succeed,
even though we did that before the last refactoring.

* Revert "event_trace_win: fix bug in ProcessInfo initialization"

This reverts commit c4ce6cf488be5489fa0a1a9c24e92dd39bbc42a1.

* Revert "#0 various windows fixes: react to PR comments"

This reverts commit cae31a25371e53a3645ddab82372c3772138d658.

* #0 various windows fixes: react to PR comments again

* keep number of allocations minimal
* add getProcessName() to retrieve the process name as char always (i.e. not UNICODE)

easy_profiler_core/block.cpp

@@ -51,7 +51,7 @@
 #include "profile_manager.h"
 #include "current_time.h"
 
-using namespace profiler;
+namespace profiler {
 
 #ifndef EASY_PROFILER_API_DISABLED
 Event::Event(timestamp_t _begin_time) : m_begin(_begin_time), m_end(0)
@@ -230,7 +230,9 @@ CSwitchEvent::CSwitchEvent(timestamp_t _begin_time, thread_id_t _tid)
 
 }
 
-CSwitchBlock::CSwitchBlock(timestamp_t _begin_time, thread_id_t _tid, const char* _runtimeName)
+} // END of namespace profiler.
+
+CSwitchBlock::CSwitchBlock(profiler::timestamp_t _begin_time, profiler::thread_id_t _tid, const char* _runtimeName)
     : CSwitchEvent(_begin_time, _tid)
     , m_name(_runtimeName)
 {

easy_profiler_core/easy_socket.cpp

@@ -106,13 +106,12 @@ int EasySocket::bind(uint16_t port)
     if (!checkSocket(m_socket))
         return -1;
 
-    struct sockaddr_in serv_addr;
+    struct sockaddr_in server_address;
-    memset(&serv_addr, 0, sizeof(serv_addr));
+    memset(&server_address, 0, sizeof(server_address));
-    serv_addr.sin_family = AF_INET;
+    server_address.sin_family = AF_INET;
-    serv_addr.sin_addr.s_addr = INADDR_ANY;
+    server_address.sin_addr.s_addr = INADDR_ANY;
-    serv_addr.sin_port = htons(port);
+    server_address.sin_port = htons(port);
-    auto res = ::bind(m_socket, (struct sockaddr*)&serv_addr, sizeof(serv_addr));
+    auto res = ::bind(m_socket, (struct sockaddr *)&server_address, sizeof(server_address));
-
     return res;
 }
 

easy_profiler_core/event_trace_win.cpp

@@ -135,6 +135,32 @@ char KERNEL_LOGGER[] = KERNEL_LOGGER_NAME;
 ::std::atomic_uint64_t TRACING_END_TIME = ATOMIC_VAR_INIT(~0ULL);
 #endif
 
+namespace {
+    /**
+     * Retrieve the process name of the given process.
+     *
+     * This method is NOT thread-safe: the returned string has to be copied somewhere before this
+     * method can be used by another thread or call.
+     *
+     * getProcessName() owns the returned string.
+     *
+     * \return a pair of the process name string and the string length.
+     */
+    std::pair<const char*, std::size_t> getProcessName(HANDLE hProcess)
+    {
+        static TCHAR buf[MAX_PATH] = {};
+        std::size_t len = static_cast<std::size_t>(GetModuleBaseName(hProcess, 0, buf, MAX_PATH));
+
+#if UNICODE
+        static char charbuf[MAX_PATH] = {};
+        std::size_t charbufLength = std::wcstombs(charbuf, buf, len);
+        return std::make_pair(charbuf, charbufLength);
+#else
+        return std::make_pair(buf, len);
+#endif
+    }
+}
+
 namespace profiler {
 
     const decltype(EVENT_DESCRIPTOR::Opcode) SWITCH_CONTEXT_OPCODE = 36;
@@ -237,14 +263,13 @@ namespace profiler {
                     auto hProc = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_VM_READ, FALSE, pid);
                     if (hProc != nullptr)
                     {
-                        static TCHAR buf[MAX_PATH] = {}; // Using static is safe because processTraceEvent() is called from one thread
+                        const auto& processName = getProcessName(hProc); // Using thread-unsafe method is safe because processTraceEvent() is called from one thread
-                        auto len = GetModuleBaseName(hProc, 0, buf, MAX_PATH);
 
-                        if (len != 0)
+                        if (processName.second != 0)
                         {
-                            pinfo->name.reserve(pinfo->name.size() + 2 + len);
+                            pinfo->name.reserve(pinfo->name.size() + 2 + processName.second);
                             pinfo->name.append(" ", 1);
-                            pinfo->name.append(buf, len);
+                            pinfo->name.append(processName.first, processName.second);
                             pinfo->valid = 1;
                         }
 
@@ -291,6 +316,17 @@ namespace profiler {
 
 	//////////////////////////////////////////////////////////////////////////
 
+	EasyEventTracer::Properties::Properties()
+	{
+#if UNICODE
+		std::wcstombs(sessionName, KERNEL_LOGGER_NAME, sizeof(sessionName));
+#else
+		std::strncpy(sessionName, KERNEL_LOGGER_NAME, sizeof(sessionName));
+#endif
+	}
+
+    //////////////////////////////////////////////////////////////////////////
+
 #ifndef EASY_MAGIC_STATIC_CPP11
     class EasyEventTracerInstance {
         friend EasyEventTracer;
@@ -328,7 +364,7 @@ namespace profiler {
         m_lowPriority.store(_value, ::std::memory_order_release);
     }
 
-    bool setPrivilege(HANDLE hToken, LPCSTR _privelegeName)
+    bool setPrivilege(HANDLE hToken, PTCHAR _privelegeName)
     {
         bool success = false;
 
@@ -409,7 +445,7 @@ namespace profiler {
                         */
 
                         // static is safe because we are guarded by spin-lock m_spin
-                        static Properties p = ([]{ Properties prp; strncpy(prp.sessionName, KERNEL_LOGGER_NAME, sizeof(prp.sessionName)); return prp; })();
+                        static Properties p;
                         p.base = m_properties.base; // Use copy of m_properties to make sure m_properties will not be changed
 
                         // Stop another session

easy_profiler_core/event_trace_win.h

@@ -81,6 +81,8 @@ namespace profiler {
 
 #pragma pack(push, 1)
         struct Properties {
+            Properties();
+
             EVENT_TRACE_PROPERTIES base;
             char sessionName[sizeof(KERNEL_LOGGER_NAME)];
         };

easy_profiler_core/profile_manager.cpp

@@ -763,6 +763,7 @@ const BaseBlockDescriptor* ProfileManager::addBlockDescriptor(EasyBlockStatus _d
     }
 #else
     auto desc = new BlockDescriptor(static_cast<block_id_t>(m_descriptors.size()), _defaultStatus, _name, _filename, _line, _block_type, _color);
+    (void)_copyName; // unused
 #endif
 
     m_descriptors.emplace_back(desc);
@@ -1178,7 +1179,7 @@ void ProfileManager::setEnabled(bool isEnable)
     guard_lock_t lock(m_dumpSpin);
 
     auto time = getCurrentTime();
-    const auto status = isEnable ? EASY_PROF_ENABLED : EASY_PROF_DISABLED;
+    const auto status = static_cast<char>(isEnable ? EASY_PROF_ENABLED : EASY_PROF_DISABLED);
     const auto prev = m_profilerStatus.exchange(status, std::memory_order_release);
     if (prev == status)
         return;

easy_profiler_core/reader.cpp

@@ -374,6 +374,20 @@ void validate_pointers(::std::atomic<int>& _progress, const char* _oldbase, ::pr
 
 //////////////////////////////////////////////////////////////////////////
 
+static bool update_progress(::std::atomic<int>& progress, int new_value, ::std::stringstream& _log)
+{
+    auto oldprogress = progress.exchange(new_value, ::std::memory_order_release);
+    if (oldprogress < 0)
+    {
+        _log << "Reading was interrupted";
+        return false;
+    }
+
+    return true;
+}
+
+//////////////////////////////////////////////////////////////////////////
+
 extern "C" {
 
     PROFILER_API ::profiler::block_index_t fillTreesFromFile(::std::atomic<int>& progress, const char* filename,
@@ -387,10 +401,8 @@ extern "C" {
                                                              bool gather_statistics,
                                                              ::std::stringstream& _log)
     {
-        auto oldprogress = progress.exchange(0, ::std::memory_order_release);
+        if (!update_progress(progress, 0, _log))
-        if (oldprogress < 0)
         {
-            _log << "Reading was interrupted";
             return 0;
         }
 
@@ -433,10 +445,8 @@ extern "C" {
     {
         EASY_FUNCTION(::profiler::colors::Cyan);
 
-        auto oldprogress = progress.exchange(0, ::std::memory_order_release);
+        if (!update_progress(progress, 0, _log))
-        if (oldprogress < 0)
         {
-            _log << "Reading was interrupted";
             return 0;
         }
 
@@ -545,11 +555,9 @@ extern "C" {
             descriptors.push_back(descriptor);
 
             i += sz;
-            auto oldprogress = progress.exchange(static_cast<int>(15 * i / descriptors_memory_size), ::std::memory_order_release);
+            if (!update_progress(progress, static_cast<int>(15 * i / descriptors_memory_size), _log))
-            if (oldprogress < 0)
             {
-                _log << "Reading was interrupted";
+                return 0;
-                return 0; // Loading interrupted
             }
         }
 
@@ -641,10 +649,8 @@ extern "C" {
                     }
                 }
 
-                auto oldprogress = progress.exchange(20 + static_cast<int>(70 * i / memory_size), ::std::memory_order_release);
+                if (!update_progress(progress, 20 + static_cast<int>(70 * i / memory_size), _log))
-                if (oldprogress < 0)
                 {
-                    _log << "Reading was interrupted";
                     return 0; // Loading interrupted
                 }
             }
@@ -759,19 +765,19 @@ extern "C" {
                                 //per_parent_statistics.reserve(tree.children.size() * 2); // this gives no speed-up on Windows
                                 // TODO: check this behavior on Linux
 
-                                for (auto i : tree.children)
+                                for (auto child_block_index : tree.children)
                                 {
-                                    auto& child = blocks[i];
+                                    auto& child = blocks[child_block_index];
-                                    child.per_parent_stats = update_statistics(per_parent_statistics, child, i, block_index, blocks);
+                                    child.per_parent_stats = update_statistics(per_parent_statistics, child, child_block_index, block_index, blocks);
                                     if (tree.depth < child.depth)
                                         tree.depth = child.depth;
                                 }
                             }
                             else
                             {
-                                for (auto i : tree.children)
+                                for (auto child_block_index : tree.children)
                                 {
-                                    const auto& child = blocks[i];
+                                    const auto& child = blocks[child_block_index];
                                     if (tree.depth < child.depth)
                                         tree.depth = child.depth;
                                 }
@@ -807,10 +813,8 @@ extern "C" {
                     }
                 }
 
-                auto oldprogress = progress.exchange(20 + static_cast<int>(70 * i / memory_size), ::std::memory_order_release);
+                if (!update_progress(progress, 20 + static_cast<int>(70 * i / memory_size), _log))
-                if (oldprogress < 0)
                 {
-                    _log << "Reading was interrupted";
                     return 0; // Loading interrupted
                 }
             }
@@ -905,9 +909,9 @@ extern "C" {
                 //});
 
                 //root.tree.shrink_to_fit();
-                for (auto i : root.children)
+                for (auto child_block_index : root.children)
                 {
-                    auto& frame = blocks[i];
+                    auto& frame = blocks[child_block_index];
 
                     if (descriptors[frame.node->id()]->type() == ::profiler::BLOCK_TYPE_BLOCK)
                         ++root.frames_number;
@@ -998,10 +1002,8 @@ extern "C" {
             descriptors.push_back(descriptor);
 
             i += sz;
-            auto oldprogress = progress.exchange(static_cast<int>(100 * i / descriptors_memory_size), ::std::memory_order_release);
+            if (!update_progress(progress, static_cast<int>(100 * i / descriptors_memory_size), _log))
-            if (oldprogress < 0)
             {
-                _log << "Reading was interrupted";
                 return false; // Loading interrupted
             }
         }

profiler_gui/blocks_tree_widget.cpp

@@ -795,7 +795,7 @@ void EasyTreeWidget::onCollapseAllClicked(bool)
 
 void EasyTreeWidget::onExpandAllClicked(bool)
 {
-    const QSignalBlocker b(this);
+    const QSignalBlocker blocker(this);
 
     m_bSilentExpandCollapse = true;
     expandAll();

profiler_gui/common_types.h

@@ -328,8 +328,6 @@ inline QString timeStringReal(TimeUnits _units, qreal _interval, int _precision
         default:
             return autoTimeStringReal(_interval, _precision);
     }
-
-    return QString();
 }
 
 inline QString timeStringRealNs(TimeUnits _units, ::profiler::timestamp_t _interval, int _precision = 1)
@@ -351,8 +349,6 @@ inline QString timeStringRealNs(TimeUnits _units, ::profiler::timestamp_t _inter
         default:
             return autoTimeStringRealNs(_interval, _precision);
     }
-
-    return QString();
 }
 
 inline QString timeStringInt(TimeUnits _units, qreal _interval)
@@ -372,8 +368,6 @@ inline QString timeStringInt(TimeUnits _units, qreal _interval)
         default:
             return autoTimeStringInt(_interval);
     }
-
-    return QString();
 }
 
 inline QString timeStringIntNs(TimeUnits _units, ::profiler::timestamp_t _interval)
@@ -393,8 +387,6 @@ inline QString timeStringIntNs(TimeUnits _units, ::profiler::timestamp_t _interv
         default:
             return autoTimeStringIntNs(_interval);
     }
-
-    return QString();
 }
 
 //////////////////////////////////////////////////////////////////////////

profiler_gui/descriptors_tree_widget.cpp

@@ -269,14 +269,14 @@ void EasyDescTreeWidget::contextMenuEvent(QContextMenuEvent* _event)
     connect(action, &QAction::triggered, this, &This::collapseAll);
 
     menu.addSeparator();
-    auto submenu = menu.addMenu("Search by");
+    auto searchby_submenu = menu.addMenu("Search by");
     auto header_item = headerItem();
     for (int i = 0; i < DESC_COL_STATUS; ++i)
     {
         if (i == DESC_COL_TYPE)
             continue;
 
-        action = submenu->addAction(header_item->text(i));
+        action = searchby_submenu->addAction(header_item->text(i));
         action->setData(i);
         action->setCheckable(true);
         if (i == m_searchColumn)
@@ -537,7 +537,7 @@ void EasyDescTreeWidget::onBlockStatusChangeClicked(bool _checked)
     }
 }
 
-void EasyDescTreeWidget::onBlockStatusChange(::profiler::block_id_t _id, ::profiler::EasyBlockStatus _status)
+void EasyDescTreeWidget::onBlockStatusChange(::profiler::block_id_t _id, ::profiler::EasyBlockStatus /* _status */)
 {
     if (m_bLocked)
         return;

profiler_gui/easy_graphics_scrollbar.cpp

@@ -154,7 +154,7 @@ EasyGraphicsSliderItem::~EasyGraphicsSliderItem()
 
 }
 
-void EasyGraphicsSliderItem::paint(QPainter* _painter, const QStyleOptionGraphicsItem* _option, QWidget* _widget)
+void EasyGraphicsSliderItem::paint(QPainter* _painter, const QStyleOptionGraphicsItem* /* _option */, QWidget* /* _widget */)
 {
     if (static_cast<const EasyGraphicsScrollbar*>(scene()->parent())->bindMode())
     {
@@ -280,7 +280,7 @@ QRectF EasyHistogramItem::boundingRect() const
     return m_boundingRect;
 }
 
-void EasyHistogramItem::paint(QPainter* _painter, const QStyleOptionGraphicsItem* _option, QWidget* _widget)
+void EasyHistogramItem::paint(QPainter* _painter, const QStyleOptionGraphicsItem* /* _option */, QWidget* /* _widget */)
 {
     if (!m_bPermitImageUpdate || (m_regime == Hist_Pointer && m_pSource == nullptr) || (m_regime == Hist_Id && (m_threadId == 0 || ::profiler_gui::is_max(m_blockId))))
         return;
@@ -1401,7 +1401,7 @@ void EasyHistogramItem::updateImage(QRectF _boundingRect, HistRegime _regime, qr
                                     qreal _minimum, qreal _maximum, qreal _range,
                                     qreal _value, qreal _width, qreal _top_duration, qreal _bottom_duration,
                                     bool _bindMode, float _frame_time, ::profiler::timestamp_t _begin_time,
-                                    qreal _origin, bool _autoAdjustHist)
+                                    qreal /* _origin */, bool _autoAdjustHist)
 {
     const auto bottom = _boundingRect.height();//_boundingRect.bottom();
     const auto screenWidth = _boundingRect.width() * _current_scale;

profiler_gui/main_window.cpp

@@ -783,14 +783,14 @@ void EasyMainWindow::loadFile(const QString& filename)
     m_reader.load(filename);
 }
 
-void EasyMainWindow::readStream(::std::stringstream& data)
+void EasyMainWindow::readStream(::std::stringstream& _data)
 {
     m_progress->setLabelText(tr("Reading from stream..."));
 
     m_progress->setValue(0);
     m_progress->show();
     m_readerTimer.start(LOADER_TIMER_INTERVAL);
-    m_reader.load(data);
+    m_reader.load(_data);
 }
 
 //////////////////////////////////////////////////////////////////////////

profiler_gui/tree_widget_item.cpp

@@ -113,8 +113,6 @@ bool EasyTreeWidgetItem::operator < (const Parent& _other) const
             return data(col, Qt::UserRole).toULongLong() < _other.data(col, Qt::UserRole).toULongLong();
         }
     }
-
-    return false;
 }
 
 ::profiler::block_index_t EasyTreeWidgetItem::block_index() const