Fix runner warnings (#8211)

.github/actions/spell-check/expect.txt

@@ -2501,6 +2501,7 @@ website
 webview
 wekyb
 wexfs
+wdupenv
 Whichdoes
 whitespaces
 WIC

src/common/logger/logger.cpp

@@ -7,6 +7,7 @@
 #include <spdlog/sinks/daily_file_sink.h>
 #include <spdlog\sinks\stdout_color_sinks-inl.h>
 #include <iostream>
+#include <spdlog\sinks\null_sink.h>
 
 using namespace std;
 using namespace spdlog;
@@ -33,31 +34,45 @@ level::level_enum getLogLevel(std::wstring_view logSettingsPath)
     return result;
 }
 
-Logger::Logger()
+std::shared_ptr<spdlog::logger> Logger::logger;
+
+bool Logger::wasLogFailedShown()
 {
+    wchar_t* pValue;
+    size_t len;
+    _wdupenv_s(&pValue, &len, logFailedShown.c_str());
+    delete[] pValue;
+    return len;
 }
 
-Logger::Logger(std::string loggerName, std::wstring logFilePath, std::wstring_view logSettingsPath)
+void Logger::init(std::string loggerName, std::wstring logFilePath, std::wstring_view logSettingsPath)
 {
     auto logLevel = getLogLevel(logSettingsPath);
     try
     {
         auto sink = make_shared<sinks::daily_file_sink_mt>(logFilePath, 0, 0, false, LogSettings::retention);
-        this->logger = make_shared<spdlog::logger>(loggerName, sink);
+        logger = make_shared<spdlog::logger>(loggerName, sink);
     }
     catch (...)
     {
-        cerr << "Can not create file logger. Create stdout logger instead" << endl;
-        this->logger = spdlog::stdout_color_mt("some_unique_name");
+        logger = spdlog::null_logger_mt(loggerName);
+        if (!wasLogFailedShown())
+        {
+            // todo: that message should be shown from init caller and strings should be localized 
+            MessageBoxW(NULL,
+                        L"Logger can not be initialized",
+                        L"PowerToys",
+                        MB_OK | MB_ICONERROR);
+
+            SetEnvironmentVariable(logFailedShown.c_str(), L"yes");
+        }
+
+        return;
     }
 
-    this->logger->set_level(logLevel);
-    this->logger->set_pattern("[%Y-%m-%d %H:%M:%S.%f] [p-%P] [t-%t] [%l] %v");
-    spdlog::register_logger(this->logger);
+    logger->set_level(logLevel);
+    logger->set_pattern("[%Y-%m-%d %H:%M:%S.%f] [p-%P] [t-%t] [%l] %v");
+    spdlog::register_logger(logger);
     spdlog::flush_every(std::chrono::seconds(3));
-}
-
-Logger::~Logger()
-{
-    this->logger.reset();
+    logger->info("{} logger is initialized", loggerName);
 }

src/common/logger/logger.h

@@ -5,53 +5,54 @@
 class Logger
 {
 private:
-    std::shared_ptr<spdlog::logger> logger;
+    inline const static std::wstring logFailedShown = L"logFailedShown";
+    static std::shared_ptr<spdlog::logger> logger;
+    static bool wasLogFailedShown();
 
 public:
-    Logger();
-    Logger(std::string loggerName, std::wstring logFilePath, std::wstring_view logSettingsPath);
+    Logger() = delete;
+
+    static void init(std::string loggerName, std::wstring logFilePath, std::wstring_view logSettingsPath);
 
     // log message should not be localized
     template<typename FormatString, typename... Args>
-    void trace(const FormatString& fmt, const Args&... args)
+    static void trace(const FormatString& fmt, const Args&... args)
     {
-        this->logger->trace(fmt, args...);
+        logger->trace(fmt, args...);
     }
 
     // log message should not be localized
     template<typename FormatString, typename... Args>
-    void debug(const FormatString& fmt, const Args&... args)
+    static void debug(const FormatString& fmt, const Args&... args)
     {
-        this->logger->debug(fmt, args...);
+        logger->debug(fmt, args...);
     }
 
     // log message should not be localized
     template<typename FormatString, typename... Args>
-    void info(const FormatString& fmt, const Args&... args)
+    static void info(const FormatString& fmt, const Args&... args)
     {
-        this->logger->info(fmt, args...);
+        logger->info(fmt, args...);
     }
 
     // log message should not be localized
     template<typename FormatString, typename... Args>
-    void warn(const FormatString& fmt, const Args&... args)
+    static void warn(const FormatString& fmt, const Args&... args)
     {
-        this->logger->warn(fmt, args...);
+        logger->warn(fmt, args...);
     }
 
     // log message should not be localized
     template<typename FormatString, typename... Args>
-    void error(const FormatString& fmt, const Args&... args)
+    static void error(const FormatString& fmt, const Args&... args)
     {
-        this->logger->error(fmt, args...);
+        logger->error(fmt, args...);
     }
 
     // log message should not be localized
     template<typename FormatString, typename... Args>
-    void critical(const FormatString& fmt, const Args&... args)
+    static void critical(const FormatString& fmt, const Args&... args)
     {
-        this->logger->critical(fmt, args...);
+        logger->critical(fmt, args...);
     }
-
-    ~Logger();
 };

src/common/logger/logger_settings.cpp

@@ -40,7 +40,6 @@ void to_file(std::wstring_view file_name, const JsonObject& obj)
     }
     catch (...)
     {
-        std::cerr << "Can not create log config file" << std::endl;
     }
 }
 
@@ -61,7 +60,6 @@ LogSettings to_settings(JsonObject jobject)
     }
     catch (...)
     {
-        std::cerr << "Can not read log level from config file" << std::endl;
         result.logLevel = LogSettings::defaultLogLevel;
     }
     

src/common/timeutil.h

@@ -51,7 +51,7 @@ namespace timeutil
 
         inline int64_t in_days(const std::time_t to, const std::time_t from)
         {
-            return static_cast<int64_t>(std::difftime(to, from) / (3600 * 24));
+            return static_cast<int64_t>(std::difftime(to, from) / (3600 * (int64_t)24));
         }
     }
 }

src/modules/colorPicker/UnitTest-ColorPickerUI/Helpers/ColorConverterTest.cs

@@ -294,7 +294,7 @@ namespace UnitTest_ColorPickerUI.Helpers
                     {
                         var color = Color.FromArgb(red, green, blue);
 
-                        Exception exception = null;
+                        Exception? exception = null;
 
                         try
                         {

src/modules/fancyzones/dll/dllmain.cpp

@@ -13,7 +13,7 @@
 #include <lib/FancyZonesData.h>
 #include <lib/FancyZonesWinHookEventIDs.h>
 #include <lib/FancyZonesData.cpp>
-#include <lib/FancyZonesLogger.h>
+#include <common/logger/logger.h>
 
 extern "C" IMAGE_DOS_HEADER __ImageBase;
 
@@ -75,7 +75,7 @@ public:
     // Enable the powertoy
     virtual void enable()
     {
-        FancyZonesLogger::GetLogger()->info("FancyZones enabling");
+        Logger::info("FancyZones enabling");
 
         if (!m_app)
         {
@@ -133,7 +133,7 @@ public:
     // Disable the powertoy
     virtual void disable()
     {
-        FancyZonesLogger::GetLogger()->info("FancyZones disabling");
+        Logger::info("FancyZones disabling");
 
         Disable(true);
     }
@@ -155,7 +155,9 @@ public:
     {
         app_name = GET_RESOURCE_STRING(IDS_FANCYZONES);
         app_key = NonLocalizable::FancyZonesStr;
-        FancyZonesLogger::Init(PTSettingsHelper::get_module_save_folder_location(app_key));
+        std::filesystem::path logFilePath(PTSettingsHelper::get_module_save_folder_location(app_key));
+        logFilePath.append(LogSettings::fancyZonesLogPath);
+        Logger::init(LogSettings::fancyZonesLoggerName, logFilePath.wstring(), PTSettingsHelper::get_log_settings_file_location());
         m_settings = MakeFancyZonesSettings(reinterpret_cast<HINSTANCE>(&__ImageBase), FancyZonesModule::get_name(), FancyZonesModule::get_key());
         FancyZonesDataInstance().LoadFancyZonesData();
         s_instance = this;

src/modules/fancyzones/lib/FancyZonesLib.vcxproj

@@ -102,7 +102,6 @@
     </ClCompile>
   </ItemDefinitionGroup>
   <ItemGroup>
-    <ClInclude Include="FancyZonesLogger.h" />
     <ClInclude Include="FancyZones.h" />
     <ClInclude Include="FancyZonesDataTypes.h" />
     <ClInclude Include="FancyZonesWinHookEventIDs.h" />
@@ -128,7 +127,6 @@
   <ItemGroup>
     <ClCompile Include="FancyZones.cpp" />
     <ClCompile Include="FancyZonesDataTypes.cpp" />
-    <ClCompile Include="FancyZonesLogger.cpp" />
     <ClCompile Include="FancyZonesWinHookEventIDs.cpp" />
     <ClCompile Include="FancyZonesData.cpp" />
     <ClCompile Include="JsonHelpers.cpp" />

src/modules/fancyzones/lib/FancyZonesLogger.cpp

@@ -1,24 +0,0 @@
-#include "pch.h"
-#include <common\settings_helpers.h>
-#include <filesystem>
-#include "FancyZonesLogger.h"
-
-std::shared_ptr<Logger> FancyZonesLogger::logger;
-
-void FancyZonesLogger::Init(std::wstring moduleSaveLocation)
-{
-    std::filesystem::path logFilePath(moduleSaveLocation);
-    logFilePath.append(LogSettings::fancyZonesLogPath);
-    logger = std::make_shared<Logger>(LogSettings::fancyZonesLoggerName, logFilePath.wstring(), PTSettingsHelper::get_log_settings_file_location());
-    logger->info("FancyZones logger initialized");
-}
-
-std::shared_ptr<Logger> FancyZonesLogger::GetLogger()
-{
-    if (!logger)
-    {
-        throw "FancyZones logger is not initialized";
-    }
-
-    return logger;
-}

src/modules/fancyzones/lib/FancyZonesLogger.h

@@ -1,10 +0,0 @@
-#pragma once
-#include <common/logger/logger.h>
-
-class FancyZonesLogger
-{
-    static std::shared_ptr<Logger> logger;
-public:
-    static void Init(std::wstring moduleSaveLocation);
-    static std::shared_ptr<Logger> GetLogger();
-};

src/modules/launcher/Microsoft.Launcher/dllmain.cpp

@@ -78,8 +78,6 @@ private:
     // Handle to event used to invoke the Runner
     HANDLE m_hEvent;
 
-    std::shared_ptr<Logger> logger;
-
 public:
     // Constructor
     Microsoft_Launcher()
@@ -88,8 +86,8 @@ public:
         app_key = LauncherConstants::ModuleKey;
         std::filesystem::path logFilePath(PTSettingsHelper::get_module_save_folder_location(this->app_key));
         logFilePath.append(LogSettings::launcherLogPath);
-        logger = std::make_shared<Logger>(LogSettings::launcherLoggerName, logFilePath.wstring(), PTSettingsHelper::get_log_settings_file_location());
-        logger->info("Launcher object is constructing");
+        Logger::init(LogSettings::launcherLoggerName, logFilePath.wstring(), PTSettingsHelper::get_log_settings_file_location());
+        Logger::info("Launcher object is constructing");
         init_settings();
 
         SECURITY_ATTRIBUTES sa;
@@ -101,8 +99,7 @@ public:
 
     ~Microsoft_Launcher()
     {
-        logger->info("Launcher object is destroying");
-        logger.reset();
+        Logger::info("Launcher object is destroying");
         if (m_enabled)
         {
             terminateProcess();
@@ -183,7 +180,7 @@ public:
     // Enable the powertoy
     virtual void enable()
     {
-        this->logger->info("Launcher is enabling");
+        Logger::info("Launcher is enabling");
         ResetEvent(m_hEvent);
         // Start PowerLauncher.exe only if the OS is 19H1 or higher
         if (UseNewSettings())
@@ -255,7 +252,7 @@ public:
     // Disable the powertoy
     virtual void disable()
     {
-        this->logger->info("Launcher is disabling");
+        Logger::info("Launcher is disabling");
         if (m_enabled)
         {
             ResetEvent(m_hEvent);
@@ -327,7 +324,7 @@ public:
         if (TerminateProcess(m_hProcess, 1) == 0)
         {
             auto err = get_last_error_message(GetLastError());
-            this->logger->error(L"Launcher process was not terminated. {}", err.has_value() ? err.value() : L"");
+            Logger::error(L"Launcher process was not terminated. {}", err.has_value() ? err.value() : L"");
         }
 
         // Temporarily disable sending a message to close

src/modules/shortcut_guide/ShortcutGuideLogger.cpp

@@ -1,24 +0,0 @@
-#include "pch.h"
-#include "ShortcutGuideLogger.h"
-#include <common\settings_helpers.h>
-#include <filesystem>
-
-std::shared_ptr<Logger> ShortcutGuideLogger::logger;
-
-void ShortcutGuideLogger::Init(std::wstring moduleSaveLocation)
-{
-    std::filesystem::path logFilePath(moduleSaveLocation);
-    logFilePath.append(LogSettings::shortcutGuideLogPath);
-    logger = std::make_shared<Logger>(LogSettings::shortcutGuideLoggerName, logFilePath.wstring(), PTSettingsHelper::get_log_settings_file_location());
-    logger->info("Shortcut Guide logger initialized");
-}
-
-std::shared_ptr<Logger> ShortcutGuideLogger::GetLogger()
-{
-    if (!logger)
-    {
-        throw "Shortcut Guide logger is not initialized";
-    }
-
-    return logger;
-}

src/modules/shortcut_guide/ShortcutGuideLogger.h

@@ -1,11 +0,0 @@
-#pragma once
-#include <common/logger/logger.h>
-
-class ShortcutGuideLogger
-{
-    static std::shared_ptr<Logger> logger;
-
-public:
-    static void Init(std::wstring moduleSaveLocation);
-    static std::shared_ptr<Logger> GetLogger();
-};

src/modules/shortcut_guide/shortcut_guide.cpp

@@ -7,9 +7,11 @@
 #include <common/settings_objects.h>
 #include <common/debug_control.h>
 #include <sstream>
-#include <modules\shortcut_guide\ShortcutGuideConstants.h>
-#include <modules\shortcut_guide\ShortcutGuideLogger.h>
-#include <common\settings_helpers.cpp>
+#include <modules/shortcut_guide/ShortcutGuideConstants.h>
+
+#include <common/settings_helpers.cpp>
+#include <common/logger/logger.h>
+
 
 extern "C" IMAGE_DOS_HEADER __ImageBase;
 
@@ -98,8 +100,10 @@ OverlayWindow::OverlayWindow()
 {
     app_name = GET_RESOURCE_STRING(IDS_SHORTCUT_GUIDE);
     app_key = ShortcutGuideConstants::ModuleKey;
-    ShortcutGuideLogger::Init(PTSettingsHelper::get_module_save_folder_location(app_key));
-    ShortcutGuideLogger::GetLogger()->info("Overlay Window is creating");
+    std::filesystem::path logFilePath(PTSettingsHelper::get_module_save_folder_location(app_key));
+    logFilePath.append(LogSettings::shortcutGuideLogPath);
+    Logger::init(LogSettings::shortcutGuideLoggerName, logFilePath.wstring(), PTSettingsHelper::get_log_settings_file_location());
+    Logger::info("Overlay Window is creating");
     init_settings();
 }
 
@@ -200,7 +204,7 @@ constexpr UINT alternative_switch_vk_code = VK_OEM_2;
 
 void OverlayWindow::enable()
 {
-    ShortcutGuideLogger::GetLogger()->info("Shortcut Guide is enabling");
+    Logger::info("Shortcut Guide is enabling");
 
     auto switcher = [&](HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) -> LRESULT {
         if (msg == WM_KEYDOWN && wparam == VK_ESCAPE && instance->target_state->active())
@@ -253,7 +257,7 @@ void OverlayWindow::enable()
 
 void OverlayWindow::disable(bool trace_event)
 {
-    ShortcutGuideLogger::GetLogger()->info("Shortcut Guide is disabling");
+    Logger::info("Shortcut Guide is disabling");
 
     if (_enabled)
     {

src/modules/shortcut_guide/shortcut_guide.vcxproj

@@ -112,7 +112,6 @@
     <ClInclude Include="Generated Files/resource.h" />
     <None Include="resource.base.h" />
     <ClInclude Include="ShortcutGuideConstants.h" />
-    <ClInclude Include="ShortcutGuideLogger.h" />
     <ClInclude Include="shortcut_guide.h" />
     <ClInclude Include="pch.h" />
     <ClInclude Include="target_state.h" />
@@ -125,7 +124,6 @@
     <ClCompile Include="overlay_window.cpp" />
     <ClCompile Include="dllmain.cpp" />
     <ClCompile Include="keyboard_state.cpp" />
-    <ClCompile Include="ShortcutGuideLogger.cpp" />
     <ClCompile Include="shortcut_guide.cpp" />
     <ClCompile Include="pch.cpp">
       <PrecompiledHeader Condition="'$(CIBuild)'!='true'">Create</PrecompiledHeader>

src/runner/main.cpp

@@ -46,7 +46,6 @@ namespace
     const wchar_t POWER_TOYS_MODULE_LOAD_FAIL[] = L"Failed to load "; // Module name will be appended on this message and it is not localized.
 }
 
-std::shared_ptr<Logger> logger;
 void chdir_current_executable()
 {
     // Change current directory to the path of the executable.
@@ -77,7 +76,11 @@ void open_menu_from_another_instance()
 
 int runner(bool isProcessElevated)
 {
-    logger->info("Runner is starting. Elevated={}", isProcessElevated);
+    std::filesystem::path logFilePath(PTSettingsHelper::get_root_save_folder_location());
+    logFilePath.append(LogSettings::runnerLogPath);
+    Logger::init(LogSettings::runnerLoggerName, logFilePath.wstring(), PTSettingsHelper::get_log_settings_file_location());
+
+    Logger::info("Runner is starting. Elevated={}", isProcessElevated);
     DPIAware::EnableDPIAwarenessForThisProcess();
 
 #if _DEBUG && _WIN64
@@ -296,10 +299,6 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine
         return 0;
     }
 
-    std::filesystem::path logFilePath(PTSettingsHelper::get_root_save_folder_location());
-    logFilePath = logFilePath.append(LogSettings::runnerLogPath);
-    logger = std::make_shared<Logger>(LogSettings::runnerLoggerName, logFilePath.wstring(), PTSettingsHelper::get_log_settings_file_location());
-
     int n_cmd_args = 0;
     LPWSTR* cmd_arg_list = CommandLineToArgvW(GetCommandLineW(), &n_cmd_args);
     switch (should_run_in_special_mode(n_cmd_args, cmd_arg_list))

src/runner/settings_window.cpp

@@ -19,6 +19,7 @@
 #include <common/os-detect.h>
 #include <common/version.h>
 #include <common/VersionHelper.h>
+#include <common/logger/logger.h>
 
 #define BUFSIZE 1024
 
@@ -276,9 +277,18 @@ void run_settings_window()
     std::wstring powertoys_pipe_name(L"\\\\.\\pipe\\powertoys_runner_");
     std::wstring settings_pipe_name(L"\\\\.\\pipe\\powertoys_settings_");
     UUID temp_uuid;
-    UuidCreate(&temp_uuid);
-    wchar_t* uuid_chars;
-    UuidToString(&temp_uuid, (RPC_WSTR*)&uuid_chars);
+    wchar_t* uuid_chars = nullptr;
+    if (UuidCreate(&temp_uuid) == RPC_S_UUID_NO_ADDRESS)
+    {
+        auto val = get_last_error_message(GetLastError());
+        Logger::warn(L"UuidCreate can not create guid. {}", val.has_value() ? val.value() : L"");
+    }
+    else if (UuidToString(&temp_uuid, (RPC_WSTR*)&uuid_chars) != RPC_S_OK)
+    {
+        auto val = get_last_error_message(GetLastError());
+        Logger::warn(L"UuidToString can not convert to string. {}", val.has_value() ? val.value() : L"");
+    }
+
     if (uuid_chars != nullptr)
     {
         powertoys_pipe_name += std::wstring(uuid_chars);
@@ -386,10 +396,18 @@ void run_settings_window()
     current_settings_ipc->start(hToken);
     g_settings_process_id = process_info.dwProcessId;
 
-    WaitForSingleObject(process_info.hProcess, INFINITE);
-    if (WaitForSingleObject(process_info.hProcess, INFINITE) != WAIT_OBJECT_0)
+    if (process_info.hProcess)
     {
-        show_last_error_message(L"Couldn't wait on the Settings Window to close.", GetLastError(), L"PowerToys - runner");
+        WaitForSingleObject(process_info.hProcess, INFINITE);
+        if (WaitForSingleObject(process_info.hProcess, INFINITE) != WAIT_OBJECT_0)
+        {
+            show_last_error_message(L"Couldn't wait on the Settings Window to close.", GetLastError(), L"PowerToys - runner");
+        }
+    }
+    else
+    {
+        auto val = get_last_error_message(GetLastError());
+        Logger::error(L"Process handle is empty. {}", val.has_value() ? val.value() : L"");
     }
 
 LExit: