diff --git a/src/config.cpp b/src/config.cpp index 86cd37a0..cefa46b4 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -7,8 +7,14 @@ #include #include #include +#include +#include +#include namespace wind { -static double clampd(double v, double lo, double hi) { return v < lo ? lo : (v > hi ? hi : v); } +static double clampd(double v, double lo, double hi) { + if (v != v) return lo; // NaN fails every comparison below and would pass through unclamped + return v < lo ? lo : (v > hi ? hi : v); +} static std::string trim(const std::string& s) { size_t a = s.find_first_not_of(" \t\r\n"); if (a == std::string::npos) return ""; @@ -16,6 +22,23 @@ static std::string trim(const std::string& s) { return s.substr(a, b - a + 1); } +// Number parsing for ini values: the whole value must be a plain decimal number. The std::sto* +// calls accepted "12abc" as 12, "0x10" as 0, and strtod-based ones take "nan", "inf" and hex floats +// (review item 72). A bad value throws, and ParseConfig keeps the default for that key. +static double ParseDoubleStrict(const std::string& v) { + if (v.empty() || v.find_first_not_of("0123456789+-.eE") != std::string::npos) + throw std::invalid_argument("not a decimal number"); + char* end = nullptr; + const double d = std::strtod(v.c_str(), &end); + if (end == v.c_str() || *end != '\0' || !std::isfinite(d)) throw std::invalid_argument("not a finite number"); + return d; +} +static int ParseIntStrict(const std::string& v) { + const double d = ParseDoubleStrict(v); + if (d < -2147483648.0 || d > 2147483647.0) throw std::out_of_range("integer out of range"); + return (int)d; +} + bool IsForbiddenBindVk(int vk) { switch (vk) { case 0x01: // VK_LBUTTON (left click) @@ -107,7 +130,8 @@ int EffectiveSamplingMode(int iniValue, bool mpoDisabledAtBoot, bool mpoDisabled Config ParseConfig(const std::string& text) { Config c; - std::istringstream in(text); + // A UTF-8 BOM (Notepad's "UTF-8 with BOM") would glue itself to the first key and lose it. + std::istringstream in(text.compare(0, 3, "\xEF\xBB\xBF") == 0 ? text.substr(3) : text); std::string line; while (std::getline(in, line)) { std::string t = trim(line); @@ -135,127 +159,127 @@ Config ParseConfig(const std::string& text) { if (key == "engineOther") { c.engineOther = val.empty() ? "auto" : val; continue; } if (val.empty()) continue; try { - if (key == "zoomInButton") c.zoomInButton = std::stoi(val); - else if (key == "zoomOutButton") c.zoomOutButton = std::stoi(val); - else if (key == "zoomInButton2") c.zoomInButton2 = std::stoi(val); - else if (key == "zoomOutButton2") c.zoomOutButton2 = std::stoi(val); - else if (key == "recenterVk") c.recenterVk = std::stoi(val); - else if (key == "cursorLockVk") c.cursorLockVk = std::stoi(val); - else if (key == "recenterMods") c.recenterMods = std::stoi(val); - else if (key == "cursorLockMods") c.cursorLockMods = std::stoi(val); - else if (key == "panLeftVk") c.panLeftVk = std::stoi(val); - else if (key == "panLeftMods") c.panLeftMods = std::stoi(val); - else if (key == "panRightVk") c.panRightVk = std::stoi(val); - else if (key == "panRightMods") c.panRightMods = std::stoi(val); - else if (key == "panUpVk") c.panUpVk = std::stoi(val); - else if (key == "panUpMods") c.panUpMods = std::stoi(val); - else if (key == "panDownVk") c.panDownVk = std::stoi(val); - else if (key == "panDownMods") c.panDownMods = std::stoi(val); - else if (key == "panSpeed") c.panSpeed = std::stod(val); - else if (key == "zoomTrace") c.zoomTrace = std::stoi(val); - else if (key == "hitchLog") c.hitchLog = std::stoi(val); - else if (key == "hitchThresholdPct") c.hitchThresholdPct = std::clamp(std::stoi(val), 110, 1000); + if (key == "zoomInButton") c.zoomInButton = ParseIntStrict(val); + else if (key == "zoomOutButton") c.zoomOutButton = ParseIntStrict(val); + else if (key == "zoomInButton2") c.zoomInButton2 = ParseIntStrict(val); + else if (key == "zoomOutButton2") c.zoomOutButton2 = ParseIntStrict(val); + else if (key == "recenterVk") c.recenterVk = ParseIntStrict(val); + else if (key == "cursorLockVk") c.cursorLockVk = ParseIntStrict(val); + else if (key == "recenterMods") c.recenterMods = ParseIntStrict(val); + else if (key == "cursorLockMods") c.cursorLockMods = ParseIntStrict(val); + else if (key == "panLeftVk") c.panLeftVk = ParseIntStrict(val); + else if (key == "panLeftMods") c.panLeftMods = ParseIntStrict(val); + else if (key == "panRightVk") c.panRightVk = ParseIntStrict(val); + else if (key == "panRightMods") c.panRightMods = ParseIntStrict(val); + else if (key == "panUpVk") c.panUpVk = ParseIntStrict(val); + else if (key == "panUpMods") c.panUpMods = ParseIntStrict(val); + else if (key == "panDownVk") c.panDownVk = ParseIntStrict(val); + else if (key == "panDownMods") c.panDownMods = ParseIntStrict(val); + else if (key == "panSpeed") c.panSpeed = ParseDoubleStrict(val); + else if (key == "zoomTrace") c.zoomTrace = ParseIntStrict(val); + else if (key == "hitchLog") c.hitchLog = ParseIntStrict(val); + else if (key == "hitchThresholdPct") c.hitchThresholdPct = std::clamp(ParseIntStrict(val), 110, 1000); // The chain is split in two: MSVC caps if/else nesting depth (C1061). Keys are unique, // so a second chain changes nothing. - if (key == "hideCursorVk") c.hideCursorVk = std::stoi(val); - else if (key == "hideCursorMods") c.hideCursorMods = std::stoi(val); - else if (key == "zoomInVk") c.zoomInVk = std::stoi(val); - else if (key == "zoomOutVk") c.zoomOutVk = std::stoi(val); - else if (key == "zoomInVk2") c.zoomInVk2 = std::stoi(val); - else if (key == "zoomOutVk2") c.zoomOutVk2 = std::stoi(val); - else if (key == "zoomInMods") c.zoomInMods = std::stoi(val); - else if (key == "zoomOutMods") c.zoomOutMods = std::stoi(val); - else if (key == "zoomInMods2") c.zoomInMods2 = std::stoi(val); - else if (key == "zoomOutMods2") c.zoomOutMods2 = std::stoi(val); - else if (key == "zoomInButtonMods") c.zoomInButtonMods = std::stoi(val); - else if (key == "zoomOutButtonMods") c.zoomOutButtonMods = std::stoi(val); - else if (key == "zoomInButton2Mods") c.zoomInButton2Mods = std::stoi(val); - else if (key == "zoomOutButton2Mods") c.zoomOutButton2Mods = std::stoi(val); - else if (key == "zoomWheelMods") c.zoomWheelMods = std::stoi(val); - else if (key == "panKeysOn") c.panKeysOn = std::stoi(val) != 0 ? 1 : 0; - else if (key == "hideCursorOn") c.hideCursorOn = std::stoi(val) != 0 ? 1 : 0; - else if (key == "cursorLockOn") c.cursorLockOn = std::stoi(val) != 0 ? 1 : 0; - else if (key == "maxLevel") c.maxLevel = std::stod(val); - else if (key == "zoomInSpeed") c.zoomInSpeed = std::stod(val); - else if (key == "zoomOutSpeed") c.zoomOutSpeed = std::stod(val); - else if (key == "smoothZoom") c.smoothZoom = std::stoi(val); - else if (key == "smoothZoomAccel") c.smoothZoomAccel = std::stod(val); - else if (key == "smoothZoomRamp") c.smoothZoomRamp = std::stod(val); - else if (key == "zoomEaseOutMs") c.zoomEaseOutMs = std::stoi(val); - else if (key == "vsync") c.vsync = std::stoi(val); - else if (key == "dwmFlush") c.dwmFlush = std::stoi(val); - else if (key == "diagnostics") c.diagnostics = std::stoi(val); - else if (key == "cursorSensitivity") c.cursorSensitivity = std::stod(val); - else if (key == "cursorSmoothing") c.cursorSmoothing = std::stod(val); - else if (key == "cursorConstantSize") c.cursorConstantSize = std::stoi(val); + if (key == "hideCursorVk") c.hideCursorVk = ParseIntStrict(val); + else if (key == "hideCursorMods") c.hideCursorMods = ParseIntStrict(val); + else if (key == "zoomInVk") c.zoomInVk = ParseIntStrict(val); + else if (key == "zoomOutVk") c.zoomOutVk = ParseIntStrict(val); + else if (key == "zoomInVk2") c.zoomInVk2 = ParseIntStrict(val); + else if (key == "zoomOutVk2") c.zoomOutVk2 = ParseIntStrict(val); + else if (key == "zoomInMods") c.zoomInMods = ParseIntStrict(val); + else if (key == "zoomOutMods") c.zoomOutMods = ParseIntStrict(val); + else if (key == "zoomInMods2") c.zoomInMods2 = ParseIntStrict(val); + else if (key == "zoomOutMods2") c.zoomOutMods2 = ParseIntStrict(val); + else if (key == "zoomInButtonMods") c.zoomInButtonMods = ParseIntStrict(val); + else if (key == "zoomOutButtonMods") c.zoomOutButtonMods = ParseIntStrict(val); + else if (key == "zoomInButton2Mods") c.zoomInButton2Mods = ParseIntStrict(val); + else if (key == "zoomOutButton2Mods") c.zoomOutButton2Mods = ParseIntStrict(val); + else if (key == "zoomWheelMods") c.zoomWheelMods = ParseIntStrict(val); + else if (key == "panKeysOn") c.panKeysOn = ParseIntStrict(val) != 0 ? 1 : 0; + else if (key == "hideCursorOn") c.hideCursorOn = ParseIntStrict(val) != 0 ? 1 : 0; + else if (key == "cursorLockOn") c.cursorLockOn = ParseIntStrict(val) != 0 ? 1 : 0; + else if (key == "maxLevel") c.maxLevel = ParseDoubleStrict(val); + else if (key == "zoomInSpeed") c.zoomInSpeed = ParseDoubleStrict(val); + else if (key == "zoomOutSpeed") c.zoomOutSpeed = ParseDoubleStrict(val); + else if (key == "smoothZoom") c.smoothZoom = ParseIntStrict(val); + else if (key == "smoothZoomAccel") c.smoothZoomAccel = ParseDoubleStrict(val); + else if (key == "smoothZoomRamp") c.smoothZoomRamp = ParseDoubleStrict(val); + else if (key == "zoomEaseOutMs") c.zoomEaseOutMs = ParseIntStrict(val); + else if (key == "vsync") c.vsync = ParseIntStrict(val); + else if (key == "dwmFlush") c.dwmFlush = ParseIntStrict(val); + else if (key == "diagnostics") c.diagnostics = ParseIntStrict(val); + else if (key == "cursorSensitivity") c.cursorSensitivity = ParseDoubleStrict(val); + else if (key == "cursorSmoothing") c.cursorSmoothing = ParseDoubleStrict(val); + else if (key == "cursorConstantSize") c.cursorConstantSize = ParseIntStrict(val); else if (key == "cursorVisibility") c.cursorVisibility = val; else if (key == "model") c.model = val; - else if (key == "fastPan") c.fastPan = std::stoi(val); - else if (key == "smoothPan") c.smoothPan = std::stoi(val); - else if (key == "magInputTransform") c.magInputTransform = std::stoi(val); - else if (key == "bilinear") c.bilinear = std::stoi(val); - else if (key == "sharpness") c.sharpness = std::stod(val); - else if (key == "zorderBand") c.zorderBand = std::stoi(val); - else if (key == "brightness") c.brightness = std::stod(val); - else if (key == "colorWarmPct") c.colorWarmPct = std::stoi(val); - else if (key == "colorDimPct") c.colorDimPct = std::stoi(val); - else if (key == "hdrTonemap") c.hdrTonemap = std::stoi(val); - else if (key == "multiMonitor") c.multiMonitor = std::stoi(val); - else if (key == "cropCapture") c.cropCapture = std::stoi(val); - else if (key == "lowGpuPriority") c.lowGpuPriority = std::stoi(val); - else if (key == "gpuPriority") c.gpuPriority = std::stoi(val); - else if (key == "gameCrop") c.gameCrop = std::stoi(val); - else if (key == "tdrTest") c.tdrTest = std::stoi(val); - else if (key == "probeClicks") c.probeClicks = std::stoi(val); - else if (key == "desktopTransform") c.desktopTransform = std::stoi(val); - else if (key == "cursorBandAuto") c.cursorBandAuto = std::stoi(val); - else if (key == "trackCaret") c.trackCaret = std::stoi(val); - else if (key == "trackFocus") c.trackFocus = std::stoi(val); - else if (key == "trackAlign") c.trackAlign = std::stoi(val); - else if (key == "mouseAlign") c.mouseAlign = std::stoi(val); - else if (key == "trackGlideMs") c.trackGlideMs = std::stoi(val); - else if (key == "trackMarginPct") c.trackMarginPct = std::stoi(val); - else if (key == "mouseMarginPct") c.mouseMarginPct = std::stoi(val); - else if (key == "trackLog") c.trackLog = std::stoi(val); - else if (key == "ixDecimate") c.ixDecimate = std::stoi(val); - else if (key == "mpoBuster") c.mpoBuster = std::stoi(val); + else if (key == "fastPan") c.fastPan = ParseIntStrict(val); + else if (key == "smoothPan") c.smoothPan = ParseIntStrict(val); + else if (key == "magInputTransform") c.magInputTransform = ParseIntStrict(val); + else if (key == "bilinear") c.bilinear = ParseIntStrict(val); + else if (key == "sharpness") c.sharpness = ParseDoubleStrict(val); + else if (key == "zorderBand") c.zorderBand = ParseIntStrict(val); + else if (key == "brightness") c.brightness = ParseDoubleStrict(val); + else if (key == "colorWarmPct") c.colorWarmPct = ParseIntStrict(val); + else if (key == "colorDimPct") c.colorDimPct = ParseIntStrict(val); + else if (key == "hdrTonemap") c.hdrTonemap = ParseIntStrict(val); + else if (key == "multiMonitor") c.multiMonitor = ParseIntStrict(val); + else if (key == "cropCapture") c.cropCapture = ParseIntStrict(val); + else if (key == "lowGpuPriority") c.lowGpuPriority = ParseIntStrict(val); + else if (key == "gpuPriority") c.gpuPriority = ParseIntStrict(val); + else if (key == "gameCrop") c.gameCrop = ParseIntStrict(val); + else if (key == "tdrTest") c.tdrTest = ParseIntStrict(val); + else if (key == "probeClicks") c.probeClicks = ParseIntStrict(val); + else if (key == "desktopTransform") c.desktopTransform = ParseIntStrict(val); + else if (key == "cursorBandAuto") c.cursorBandAuto = ParseIntStrict(val); + else if (key == "trackCaret") c.trackCaret = ParseIntStrict(val); + else if (key == "trackFocus") c.trackFocus = ParseIntStrict(val); + else if (key == "trackAlign") c.trackAlign = ParseIntStrict(val); + else if (key == "mouseAlign") c.mouseAlign = ParseIntStrict(val); + else if (key == "trackGlideMs") c.trackGlideMs = ParseIntStrict(val); + else if (key == "trackMarginPct") c.trackMarginPct = ParseIntStrict(val); + else if (key == "mouseMarginPct") c.mouseMarginPct = ParseIntStrict(val); + else if (key == "trackLog") c.trackLog = ParseIntStrict(val); + else if (key == "ixDecimate") c.ixDecimate = ParseIntStrict(val); + else if (key == "mpoBuster") c.mpoBuster = ParseIntStrict(val); else if (key == "txSamplingMode") { // Only 0 (nearest), 1 (smooth) and -1 (leave DWM alone) are meaningful. 2..4 are // kernel aliases of nearest that would slip past the MPO sampling guard; anything // else folds to nearest so EffectiveSamplingMode guards it (review 2026-10-09 #23). - const int v = std::stoi(val); + const int v = ParseIntStrict(val); c.txSamplingMode = (v == 1 || v == -1) ? v : 0; } - else if (key == "txWarmMode") c.txWarmMode = std::stoi(val); - else if (key == "txTrace") c.txTrace = std::stoi(val); - else if (key == "txRestLevel") c.txRestLevel = std::stod(val); - else if (key == "launchQuiesce") c.launchQuiesce = std::stoi(val); - else if (key == "txWarmHz") c.txWarmHz = std::stoi(val); - else if (key == "mpoNearestGuard") c.mpoNearestGuard = std::stoi(val); - else if (key == "txSmoothLadder") c.txSmoothLadder = std::stoi(val); - else if (key == "mpoGuardTest") c.mpoGuardTest = std::stoi(val); - else if (key == "mpoGuard") c.mpoGuard = std::stoi(val); - else if (key == "mpoGuardLiftWall") c.mpoGuardLiftWall = std::stoi(val); - else if (key == "lockedBallistics") c.lockedBallistics = std::stoi(val); - else if (key == "edgeClip") c.edgeClip = std::stoi(val); - else if (key == "txMaxStepPct") c.txMaxStepPct = std::stoi(val); - else if (key == "warpLock") c.warpLock = std::stoi(val); - else if (key == "lockForce") c.lockForce = std::stoi(val); - else if (key == "txEdgeMargin") c.txEdgeMargin = std::stod(val); - else if (key == "gameFpsCap") c.gameFpsCap = std::stoi(val); - else if (key == "onboarded") c.onboarded = std::stoi(val); - else if (key == "quickZoomDefault") c.quickZoomDefault = std::stod(val); + else if (key == "txWarmMode") c.txWarmMode = ParseIntStrict(val); + else if (key == "txTrace") c.txTrace = ParseIntStrict(val); + else if (key == "txRestLevel") c.txRestLevel = ParseDoubleStrict(val); + else if (key == "launchQuiesce") c.launchQuiesce = ParseIntStrict(val); + else if (key == "txWarmHz") c.txWarmHz = ParseIntStrict(val); + else if (key == "mpoNearestGuard") c.mpoNearestGuard = ParseIntStrict(val); + else if (key == "txSmoothLadder") c.txSmoothLadder = ParseIntStrict(val); + else if (key == "mpoGuardTest") c.mpoGuardTest = ParseIntStrict(val); + else if (key == "mpoGuard") c.mpoGuard = ParseIntStrict(val); + else if (key == "mpoGuardLiftWall") c.mpoGuardLiftWall = ParseIntStrict(val); + else if (key == "lockedBallistics") c.lockedBallistics = ParseIntStrict(val); + else if (key == "edgeClip") c.edgeClip = ParseIntStrict(val); + else if (key == "txMaxStepPct") c.txMaxStepPct = ParseIntStrict(val); + else if (key == "warpLock") c.warpLock = ParseIntStrict(val); + else if (key == "lockForce") c.lockForce = ParseIntStrict(val); + else if (key == "txEdgeMargin") c.txEdgeMargin = ParseDoubleStrict(val); + else if (key == "gameFpsCap") c.gameFpsCap = ParseIntStrict(val); + else if (key == "onboarded") c.onboarded = ParseIntStrict(val); + else if (key == "quickZoomDefault") c.quickZoomDefault = ParseDoubleStrict(val); else if (key == "quickZoomModifier") c.quickZoomModifier = val; - else if (key == "quickZoomHotkeyMode") c.quickZoomHotkeyMode = std::stoi(val); - else if (key == "quickZoomVk") c.quickZoomVk = std::stoi(val); - else if (key == "quickZoomMods") c.quickZoomMods = std::stoi(val); - else if (key == "outline") c.outline = std::stoi(val); - else if (key == "outlineThickness") c.outlineThickness = std::stoi(val); + else if (key == "quickZoomHotkeyMode") c.quickZoomHotkeyMode = ParseIntStrict(val); + else if (key == "quickZoomVk") c.quickZoomVk = ParseIntStrict(val); + else if (key == "quickZoomMods") c.quickZoomMods = ParseIntStrict(val); + else if (key == "outline") c.outline = ParseIntStrict(val); + else if (key == "outlineThickness") c.outlineThickness = ParseIntStrict(val); else if (key == "outlineColor") { c.outlineColor = val; ParseHexColor(val, c.outlineR, c.outlineG, c.outlineB); } - else if (key == "outlineLowZoomOnly") c.outlineLowZoomOnly = std::stoi(val); - else if (key == "outlineLowZoomMax") c.outlineLowZoomMax = std::stod(val); - else if (key == "outlineIdleHide") c.outlineIdleHide = std::stoi(val); - else if (key == "outlineIdleSeconds") c.outlineIdleSeconds = std::stod(val); + else if (key == "outlineLowZoomOnly") c.outlineLowZoomOnly = ParseIntStrict(val); + else if (key == "outlineLowZoomMax") c.outlineLowZoomMax = ParseDoubleStrict(val); + else if (key == "outlineIdleHide") c.outlineIdleHide = ParseIntStrict(val); + else if (key == "outlineIdleSeconds") c.outlineIdleSeconds = ParseDoubleStrict(val); } catch (...) { /* keep default on bad value */ } } // Clamp numeric fields to their documented ranges. The ini is a hand-editable surface, and an @@ -280,6 +304,10 @@ Config ParseConfig(const std::string& text) { if (c.outlineThickness < 1) c.outlineThickness = 1; if (c.outlineThickness > 40) c.outlineThickness = 40; c.outlineLowZoomMax = clampd(c.outlineLowZoomMax, 1.0, 50.0); + if (c.trackGlideMs < 0) c.trackGlideMs = 0; // glide time; a negative one would run the spring backwards + if (c.trackGlideMs > 5000) c.trackGlideMs = 5000; + if (c.txMaxStepPct < 0) c.txMaxStepPct = 0; // 0 = uncapped + if (c.txMaxStepPct > 1000) c.txMaxStepPct = 1000; if (c.gameFpsCap < 0) c.gameFpsCap = 0; // 0 = off if (c.gameFpsCap > 240) c.gameFpsCap = 240; if (c.gpuPriority < -1) c.gpuPriority = -1; // tri-state: -1 low / 0 normal / +1 high diff --git a/src/config_path.h b/src/config_path.h index 48995f60..cb3e32f6 100644 --- a/src/config_path.h +++ b/src/config_path.h @@ -13,7 +13,22 @@ namespace wind { +// Writability probe: a create-and-delete-on-close sentinel with a PER-PROCESS name. The old shared +// ".windwritetest" opened with share mode 0 failed for the second of two processes starting at the +// same moment, which then fell back to %LOCALAPPDATA% and split the ini and logs (review item 74). +inline bool DirIsWritable(const std::wstring& dir) { + std::wstring sentinel = dir + L"\\.windwritetest." + std::to_wstring(GetCurrentProcessId()); + HANDLE h = CreateFileW(sentinel.c_str(), GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL | FILE_FLAG_DELETE_ON_CLOSE, nullptr); + if (h == INVALID_HANDLE_VALUE) return false; + CloseHandle(h); + return true; +} + +// Resolved once per process: the exe location and its writability do not change, and every caller +// (tick reload, settings, tray) would otherwise repeat the probe and the seed check. inline std::wstring ResolveIniPath() { + static const std::wstring cached = [] { wchar_t exePathBuf[MAX_PATH]; GetModuleFileNameW(nullptr, exePathBuf, MAX_PATH); wchar_t* slash = wcsrchr(exePathBuf, L'\\'); @@ -21,13 +36,8 @@ inline std::wstring ResolveIniPath() { std::wstring exeDir(exePathBuf); std::wstring exeIni = exeDir + L"\\magnifier.ini"; - // Write a sentinel file that is auto-deleted on close, to probe writability without leaving a - // trace. If this succeeds the exe dir is fine (dev / portable install). - std::wstring sentinel = exeDir + L"\\.windwritetest"; - HANDLE h = CreateFileW(sentinel.c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, - FILE_ATTRIBUTE_NORMAL | FILE_FLAG_DELETE_ON_CLOSE, nullptr); - bool exeDirWritable = (h != INVALID_HANDLE_VALUE); - if (exeDirWritable) { CloseHandle(h); return exeIni; } + // If the probe succeeds the exe dir is fine (dev / portable install). + if (DirIsWritable(exeDir)) return exeIni; // Read-only install (typically C:\Program Files\Wind). Fall back to %LOCALAPPDATA%\Wind. wchar_t buf[MAX_PATH]; @@ -44,6 +54,8 @@ inline std::wstring ResolveIniPath() { CopyFileW(exeIni.c_str(), lapIni.c_str(), FALSE); } return lapIni; + }(); + return cached; } // Directory for logs + crash dumps. Mirrors ResolveIniPath: exe dir if writable (dev/portable), @@ -59,11 +71,8 @@ inline std::wstring ResolveLogDir() { if (slash) *slash = L'\0'; std::wstring exeDir(exePathBuf); - std::wstring sentinel = exeDir + L"\\.windwritetest"; - HANDLE h = CreateFileW(sentinel.c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, - FILE_ATTRIBUTE_NORMAL | FILE_FLAG_DELETE_ON_CLOSE, nullptr); std::wstring base; - if (h != INVALID_HANDLE_VALUE) { CloseHandle(h); base = exeDir; } + if (DirIsWritable(exeDir)) base = exeDir; else { wchar_t buf[MAX_PATH]; DWORD n = GetEnvironmentVariableW(L"LOCALAPPDATA", buf, MAX_PATH); diff --git a/src/config_ui/main.cpp b/src/config_ui/main.cpp index 1cd0cca2..ae10f87e 100644 --- a/src/config_ui/main.cpp +++ b/src/config_ui/main.cpp @@ -237,6 +237,8 @@ static std::string DoSwitchProfile(const std::string& name, bool mirrorOutgoing std::string profText; if (!wind::ReadTextFileOk(pp, profText)) return "Could not read the profile file"; { std::string terr = wind::ProfileTextError(profText); if (!terr.empty()) return terr; } + // Held to the end: the whole read-modify-write (and the rollback) is one unit (review item 71). + wind::IniWriteLock iniLock; std::string oldLive; if (!wind::ReadLiveIni(IniPath(), oldLive)) return "Could not read the config file"; // Capture hand edits (openIni) into the outgoing profile before the live ini is replaced. @@ -397,8 +399,7 @@ static void HandleWebMessage(ICoreWebView2* wv, const std::wstring& jsonW) { // showed the new value while the ini kept the old one. Tell the page, which says so. // setConfig writes the live ini (the session) only; the profile file changes on Save. std::string live; - if (!wind::ReadLiveIni(IniPath(), live) || - !WriteFileAtomic(IniPath(), wind::UpdateIniText(live, key, value))) { + if (wind::UpdateIniKeys(IniPath(), { { key, value } }, &live) != wind::IniUpdate::Ok) { wind::Log(wind::LogLevel::Warn, "config", "setConfig: writing %s=%s failed", key.c_str(), value.c_str()); wv->PostWebMessageAsJson( @@ -410,7 +411,7 @@ static void HandleWebMessage(ICoreWebView2* wv, const std::wstring& jsonW) { // Keybind captures: also written straight into the active profile so they survive // a later Discard or restart while other changes stay unsaved. Only this one key // moves; the rest of the profile file stays as saved. - const std::string active = ActiveProfileName(wind::UpdateIniText(live, key, value)); + const std::string active = ActiveProfileName(live); std::string prof; if (!active.empty() && SafeName(active) && GetFileAttributesW(ProfilePath(active).c_str()) != INVALID_FILE_ATTRIBUTES && @@ -431,6 +432,7 @@ static void HandleWebMessage(ICoreWebView2* wv, const std::wstring& jsonW) { Widen(std::string("{\"type\":\"sessionSaved\",\"ok\":") + (ok ? "true" : "false") + "}").c_str()); } else if (type == "discardSession") { // Discard: the live ini goes back to the saved profile (globals kept); reply with fresh state. + wind::IniWriteLock iniLock; // item 71 std::string live; if (ReadIni(live)) { const std::string active = ActiveProfileName(live); @@ -542,9 +544,8 @@ static void HandleWebMessage(ICoreWebView2* wv, const std::wstring& jsonW) { PostProfiles(wv, err.empty(), err); } else if (type == "createProfile") { // Factory defaults by design (spec): absent keys fall back to built-in defaults. `model` is - // seeded EXPLICITLY because the UI schema default (hybrid/"Auto") and the core's missing-key - // default (render) disagree; writing the documented product default keeps core, host, tray, - // and UI in agreement. Globals (onboarded=1, uiPalette, showAdvanced) carry over in MakeLiveText. + // seeded EXPLICITLY so a fresh profile file always names its engine (the + // core's own missing-key default is hybrid, the same as the UI schema's "Auto"). Globals (onboarded=1, uiPalette, showAdvanced) carry over in MakeLiveText. // fromCurrent ("Duplicate current"): the new profile is the live session as it stands, unsaved // changes included, built here from the live text so `model` (and everything else) is exactly // what is running: no restart, no write-back by the page, and the outgoing profile is NOT @@ -593,6 +594,7 @@ static void HandleWebMessage(ICoreWebView2* wv, const std::wstring& jsonW) { if (err.empty() && wind::SameProfileName(vals["profile"], from)) { // The pointer update must land or the live ini names a file that no longer exists; // verify the write and roll the rename back if it failed. + wind::IniWriteLock iniLock; // item 71 std::string live; if (!wind::ReadLiveIni(IniPath(), live) || !wind::WriteTextFileAtomic(IniPath(), wind::UpdateIniText(live, "profile", to))) { diff --git a/src/logging.cpp b/src/logging.cpp index 66a6e2a8..1a16bacd 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -135,12 +135,17 @@ namespace { } // wind-.log -> wind-.1.log -> wind-.2.log; oldest dropped. - void RotateIfNeeded(const std::wstring& dir, const std::wstring& stem) { + // Returns false only when the base log is over the cap and could NOT be moved aside (a tailer + // without FILE_SHARE_DELETE holds it): the caller keeps appending and backs off (review item 68). + // The base moves to a side name FIRST, so a refused move leaves every older generation intact. + bool RotateIfNeeded(const std::wstring& dir, const std::wstring& stem) { std::wstring base = dir + L"\\" + stem + L".log"; WIN32_FILE_ATTRIBUTE_DATA d{}; - if (!GetFileAttributesExW(base.c_str(), GetFileExInfoStandard, &d)) return; + if (!GetFileAttributesExW(base.c_str(), GetFileExInfoStandard, &d)) return true; ULARGE_INTEGER sz; sz.LowPart = d.nFileSizeLow; sz.HighPart = d.nFileSizeHigh; - if (!ShouldRotate(sz.QuadPart, kLogMaxBytes)) return; + if (!ShouldRotate(sz.QuadPart, kLogMaxBytes)) return true; + std::wstring side = dir + L"\\" + stem + L".rotating.log"; + if (!MoveFileExW(base.c_str(), side.c_str(), MOVEFILE_REPLACE_EXISTING)) return false; // Drop the oldest, shift the rest up by one generation. std::wstring oldest = dir + L"\\" + stem + L"." + std::to_wstring(kLogGenerations - 1) + L".log"; DeleteFileW(oldest.c_str()); @@ -150,7 +155,9 @@ namespace { MoveFileExW(from.c_str(), to.c_str(), MOVEFILE_REPLACE_EXISTING); } std::wstring to1 = dir + L"\\" + stem + L".1.log"; - MoveFileExW(base.c_str(), to1.c_str(), MOVEFILE_REPLACE_EXISTING); + if (!MoveFileExW(side.c_str(), to1.c_str(), MOVEFILE_REPLACE_EXISTING)) + DeleteFileW(side.c_str()); // .1 is held open: drop the old data rather than leave it behind + return true; } } // namespace @@ -212,20 +219,32 @@ static void PruneStrayPidLogs(const std::wstring& dir) { // by or a LogFlush asked, rotates at the cap. Below normal priority: nothing waits on it except // LogFlush callers (export, crash, shutdown). static void WriterAppend(const std::string& batch) { - if (batch.empty() || g_logFile == INVALID_HANDLE_VALUE) return; + if (batch.empty()) return; + if (g_logFile == INVALID_HANDLE_VALUE) { // a failed reopen after rotation: retry, do not stay silent + g_logFile = CreateFileW(g_logPath.c_str(), FILE_APPEND_DATA, FILE_SHARE_READ, + nullptr, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr); + if (g_logFile == INVALID_HANDLE_VALUE) return; + } DWORD wrote = 0; WriteFile(g_logFile, batch.data(), (DWORD)batch.size(), &wrote, nullptr); g_fileBytes += wrote; } static void WriterRotateIfNeeded() { - if (!g_ownsBase || !ShouldRotate(g_fileBytes, kLogMaxBytes)) return; + if (!g_ownsBase || g_logFile == INVALID_HANDLE_VALUE || !ShouldRotate(g_fileBytes, kLogMaxBytes)) return; FlushFileBuffers(g_logFile); CloseHandle(g_logFile); - RotateIfNeeded(g_logDir, g_logStem); + const bool moved = RotateIfNeeded(g_logDir, g_logStem); g_logFile = CreateFileW(g_logPath.c_str(), FILE_APPEND_DATA, FILE_SHARE_READ, nullptr, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr); g_fileBytes = 0; + if (!moved && g_logFile != INVALID_HANDLE_VALUE) { + // The base could not be moved aside: it is still the live log. Count its real size less one + // cap's worth, so the next attempt comes after another cap of lines, not on every pass. + LARGE_INTEGER sz{}; + if (GetFileSizeEx(g_logFile, &sz) && (unsigned long long)sz.QuadPart > kLogMaxBytes) + g_fileBytes = (unsigned long long)sz.QuadPart - kLogMaxBytes; + } } // Move a per-PID fallback log onto the shared base log once this process owns the app (review @@ -596,7 +615,10 @@ bool ZipLogDir(const wchar_t* destZipPath) { auto psQuote = [](const std::wstring& s) { std::wstring out = L"'"; - for (wchar_t c : s) { if (c == L'\'') out += L"''"; else out += c; } + // PowerShell reads the typographic quotes U+2018-201B as single quotes too: double them all. + for (wchar_t c : s) { + if (c == L'\'' || (c >= 0x2018 && c <= 0x201B)) { out += c; out += c; } else out += c; + } out += L"'"; return out; }; diff --git a/src/main.cpp b/src/main.cpp index 40270470..95451e21 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3261,7 +3261,20 @@ int WINAPI wWinMain(HINSTANCE hInst, HINSTANCE, PWSTR, int) { if (!(GetEnvironmentVariableA("WIND_TESTLOG", tlPath, sizeof(tlPath)) > 0 && tlPath[0])) { char ctl[512] = {}; if (ExpandEnvironmentStringsA("%LOCALAPPDATA%\\Wind\\testlog.txt", ctl, sizeof(ctl)) > 0) { - if (FILE* cf = fopen(ctl, "rb")) { + // Ignore a control file older than a day: a leftover would record telemetry on every launch. + WIN32_FILE_ATTRIBUTE_DATA cfa{}; + bool fresh = true; + if (GetFileAttributesExA(ctl, GetFileExInfoStandard, &cfa)) { + FILETIME nowFt; GetSystemTimeAsFileTime(&nowFt); + ULARGE_INTEGER a, b; + a.LowPart = nowFt.dwLowDateTime; a.HighPart = nowFt.dwHighDateTime; + b.LowPart = cfa.ftLastWriteTime.dwLowDateTime; b.HighPart = cfa.ftLastWriteTime.dwHighDateTime; + const long long ageSec = ((long long)a.QuadPart - (long long)b.QuadPart) / 10000000LL; + fresh = wind::TestlogControlFresh(ageSec); + if (!fresh) + wind::Log(wind::LogLevel::Warn, "test", "ignoring stale testlog.txt (%lld s old)", ageSec); + } + if (fresh) if (FILE* cf = fopen(ctl, "rb")) { size_t n = fread(tlPath, 1, sizeof(tlPath) - 1, cf); fclose(cf); while (n > 0 && (tlPath[n - 1] == '\r' || tlPath[n - 1] == '\n' || @@ -3272,7 +3285,7 @@ int WINAPI wWinMain(HINSTANCE hInst, HINSTANCE, PWSTR, int) { } } if (tlPath[0] && g_testlog.open(tlPath)) - wind::Log(wind::LogLevel::Info, "test", "telemetry -> %s", tlPath); + wind::Log(wind::LogLevel::Warn, "test", "telemetry -> %s", tlPath); } // Auto-detect the display refresh rate so we never assume a fixed rate (the dev's 144Hz). diff --git a/src/profiles.cpp b/src/profiles.cpp index 1d68b7bc..a167b827 100644 --- a/src/profiles.cpp +++ b/src/profiles.cpp @@ -22,7 +22,10 @@ bool IsGlobalProfileKey(const std::string& key) { } std::string ProfileNameError(const std::string& name) { if (name.empty()) return "Name cannot be empty"; - if (name.size() > 40) return "Name is too long (max 40 characters)"; + // 40 characters, not bytes: count UTF-8 lead bytes so a non-ASCII name gets the same room as in the UI. + size_t chars = 0; + for (unsigned char c : name) if ((c & 0xC0) != 0x80) ++chars; + if (chars > 40) return "Name is too long (max 40 characters)"; for (unsigned char c : name) { if (c < 0x20) return "Name contains a control character"; if (std::string("\\/:*?\"<>|").find((char)c) != std::string::npos) @@ -33,8 +36,10 @@ std::string ProfileNameError(const std::string& name) { static const char* reserved[] = {"con","prn","aux","nul", "com1","com2","com3","com4","com5","com6","com7","com8","com9", "lpt1","lpt2","lpt3","lpt4","lpt5","lpt6","lpt7","lpt8","lpt9"}; - const std::string l = lower(name); - for (const char* r : reserved) if (l == r) return "That name is reserved by Windows"; + // Windows reserves the device name with ANY extension too ("con.x", "NUL.txt", "com1 .ini"). + std::string stem = lower(name.substr(0, name.find('.'))); + while (!stem.empty() && stem.back() == ' ') stem.pop_back(); + for (const char* r : reserved) if (stem == r) return "That name is reserved by Windows"; return ""; } bool ProfileNameTaken(const std::string& name, const std::vector& names) { @@ -112,14 +117,19 @@ bool SessionDiffers(const std::string& liveText, const std::string& profileText) for (auto it = b.begin(); it != b.end();) it = IsGlobalProfileKey(it->first) ? b.erase(it) : std::next(it); // A key one side lacks reads as the built-in default (the first-run template carries each // templated key's default), so a tray drag that wrote an explicit default back (Warmth 0 -> 40 - // -> 0) is not "unsaved". Keys outside the template stay missing-vs-present. "model" is left - // out: the core's missing-key engine and the template's explicit one are different questions - // (see createProfile in config_ui/main.cpp). - static const std::map defaults = ReadIniValues(DefaultIniText()); + // -> 0) is not "unsaved". The UI keys the template omits take theirs from Config itself, so + // dragging "Release glide" back to 45 is not "unsaved" either (review item 70). + static const std::map defaults = [] { + std::map d = ReadIniValues(DefaultIniText()); + const Config c; + d.emplace("txSamplingMode", std::to_string(c.txSamplingMode)); + d.emplace("zoomEaseOutMs", std::to_string(c.zoomEaseOutMs)); + d.emplace("lockApps", c.lockApps); + return d; + }(); auto valueOf = [&](const std::map& m, const std::string& k, std::string& out) { auto it = m.find(k); if (it != m.end()) { out = it->second; return true; } - if (k == "model") return false; auto d = defaults.find(k); if (d == defaults.end()) return false; out = d->second; diff --git a/src/profiles_io.h b/src/profiles_io.h index b897947c..852c175e 100644 --- a/src/profiles_io.h +++ b/src/profiles_io.h @@ -7,6 +7,7 @@ #include #include #include +#include #include "profiles.h" #include "config_ui/ini_edit.h" #include "config.h" // MigrateWheelMods (pure) @@ -134,6 +135,42 @@ inline bool WriteTextFileAtomic(const std::wstring& path, const std::string& tex return false; } } +// Cross-process lock for read-modify-write of the live ini (review item 71). Settings, the tray flyout +// and the engine pick all read the ini, change a key and replace the file; two of them interleaving +// lost one write. A named mutex serialises them. A timeout proceeds unlocked (no worse than before); +// an abandoned mutex (a writer died holding it) is simply taken over. +class IniWriteLock { +public: + explicit IniWriteLock(DWORD waitMs = 3000) { + h_ = CreateMutexW(nullptr, FALSE, L"Local\\Wind_IniWrite"); + if (h_) { + const DWORD r = WaitForSingleObject(h_, waitMs); + owned_ = (r == WAIT_OBJECT_0 || r == WAIT_ABANDONED); + } + } + ~IniWriteLock() { + if (h_) { if (owned_) ReleaseMutex(h_); CloseHandle(h_); } + } + IniWriteLock(const IniWriteLock&) = delete; + IniWriteLock& operator=(const IniWriteLock&) = delete; +private: + HANDLE h_ = nullptr; + bool owned_ = false; +}; +enum class IniUpdate { Ok, Unreadable, WriteFailed }; +// Locked read-modify-write of the live ini: sets every key in `kv`. Unreadable = the file exists but +// could not be read (nothing written); WriteFailed = the atomic replace failed. If `outText` is given +// it receives the new text on success. +inline IniUpdate UpdateIniKeys(const std::wstring& path, const std::map& kv, + std::string* outText = nullptr) { + IniWriteLock lock; + std::string text; + if (!ReadLiveIni(path, text)) return IniUpdate::Unreadable; + for (const auto& e : kv) text = UpdateIniText(text, e.first, e.second); + if (!WriteTextFileAtomic(path, text)) return IniUpdate::WriteFailed; + if (outText) *outText = text; + return IniUpdate::Ok; +} // Sweep the temp files a killed process left behind (WriteTextFileAtomic's ".ini..tmp"): // the installer force-kills WindConfig.exe and Wind can crash between the write and the rename. // Looks in the ini's folder and in profiles\. A live writer's temp exists for milliseconds, so diff --git a/src/test_telemetry.h b/src/test_telemetry.h index a74f28b1..bfd0b0b3 100644 --- a/src/test_telemetry.h +++ b/src/test_telemetry.h @@ -46,6 +46,11 @@ inline int FormatTelemetryLine(char* buf, int cap, const TelemetrySample& s) { return (n > 0 && n < cap) ? n : 0; } +// The testlog.txt control file is written by the harness right before a launch and deleted after +// the suite. One older than a day is a leftover: honouring it would record per-tick CSV (about +// 50 MB/h) on every launch forever (review item 69). A negative age (clock skew) counts as fresh. +inline bool TestlogControlFresh(long long ageSeconds) { return ageSeconds < 24LL * 3600LL; } + // Buffered appender. Lines are cheap (~70 bytes); flush every kFlushLines so a crash mid-run // loses at most a fraction of a second of samples and the harness can tail the file live. class TestTelemetry { @@ -58,14 +63,15 @@ class TestTelemetry { if (!f_) return false; std::fputs(TelemetryHeader(), f_); lines_ = 0; + bytes_ = 0; return true; } bool enabled() const { return f_ != nullptr; } void write(const TelemetrySample& s) { - if (!f_) return; + if (!f_ || bytes_ >= kMaxBytes) return; // size cap: a forgotten opt-in cannot fill the disk char buf[192]; const int n = FormatTelemetryLine(buf, (int)sizeof(buf), s); - if (n > 0) std::fwrite(buf, 1, (size_t)n, f_); + if (n > 0) { std::fwrite(buf, 1, (size_t)n, f_); bytes_ += (unsigned long long)n; } if (++lines_ >= kFlushLines) { std::fflush(f_); lines_ = 0; } } void close() { @@ -73,8 +79,10 @@ class TestTelemetry { } private: static constexpr int kFlushLines = 32; + static constexpr unsigned long long kMaxBytes = 64ULL * 1024ULL * 1024ULL; std::FILE* f_ = nullptr; int lines_ = 0; + unsigned long long bytes_ = 0; }; } // namespace wind diff --git a/src/tray_app/engine_dropdown.cpp b/src/tray_app/engine_dropdown.cpp index 41815f86..8ae1e9ae 100644 --- a/src/tray_app/engine_dropdown.cpp +++ b/src/tray_app/engine_dropdown.cpp @@ -28,23 +28,32 @@ void WriteSessionKeep() { } } // namespace -bool SetMainEngine(const std::wstring& ini, int picked) { - std::string live; - if (GetFileAttributesW(ini.c_str()) != INVALID_FILE_ATTRIBUTES && !wind::ReadTextFileOk(ini, live)) { - wind::Log(wind::LogLevel::Warn, "tray", "engine pick: ini unreadable (err=%lu)", GetLastError()); - Notify(L"Wind", L"Could not change the engine (config file is locked)."); - return false; - } - const IniValues vals = wind::ReadIniValues(live); - auto mi = vals.find(Flyout::kEngineKey); - const std::string oldModel = mi == vals.end() ? std::string() : mi->second; - if (!Flyout::EnginePickChanges(Flyout::EngineIndex(oldModel), picked)) return false; - +bool SetMainEngine(const std::wstring& ini, int picked, const std::map& carry) { + std::string oldModel; const char* value = Flyout::EngineValue(picked); - if (!wind::WriteTextFileAtomic(ini, wind::UpdateIniText(live, Flyout::kEngineKey, value))) { - wind::Log(wind::LogLevel::Warn, "tray", "engine pick: ini write failed (err=%lu)", GetLastError()); - Notify(L"Wind", L"Could not change the engine (config file is locked)."); - return false; + { + // The whole read-modify-write under the shared ini lock, so a Settings write cannot slip in + // between and be lost (review item 71). + wind::IniWriteLock lock; + std::string live; + if (!wind::ReadLiveIni(ini, live)) { + wind::Log(wind::LogLevel::Warn, "tray", "engine pick: ini unreadable (err=%lu)", GetLastError()); + Notify(L"Wind", L"Could not change the engine (config file is locked)."); + return false; + } + const IniValues vals = wind::ReadIniValues(live); + auto mi = vals.find(Flyout::kEngineKey); + oldModel = mi == vals.end() ? std::string() : mi->second; + for (const auto& kv : carry) live = wind::UpdateIniText(live, kv.first, kv.second); + if (!Flyout::EnginePickChanges(Flyout::EngineIndex(oldModel), picked)) { + if (!carry.empty()) wind::WriteTextFileAtomic(ini, live); + return false; + } + if (!wind::WriteTextFileAtomic(ini, wind::UpdateIniText(live, Flyout::kEngineKey, value))) { + wind::Log(wind::LogLevel::Warn, "tray", "engine pick: ini write failed (err=%lu)", GetLastError()); + Notify(L"Wind", L"Could not change the engine (config file is locked)."); + return false; + } } wind::Log(wind::LogLevel::Info, "tray", "engine pick: model %s -> %s (restarting Wind)", oldModel.empty() ? "(unset)" : oldModel.c_str(), value); @@ -63,6 +72,7 @@ bool SetMainEngine(const std::wstring& ini, int picked) { wind::Log(wind::LogLevel::Warn, "tray", "engine pick: relaunch FAILED (rc=%lld haveExe=%d); reverting model", static_cast(rc), (int)haveExe); DeleteFileW(wind::SessionKeepPath().c_str()); + wind::IniWriteLock iniLock; // item 71 std::string cur; if (wind::ReadLiveIni(ini, cur)) wind::WriteTextFileAtomic(ini, oldModel.empty() ? wind::UpdateIniText(cur, Flyout::kEngineKey, "hybrid") diff --git a/src/tray_app/flyout_window.cpp b/src/tray_app/flyout_window.cpp index 1abacb4a..b4c79eba 100644 --- a/src/tray_app/flyout_window.cpp +++ b/src/tray_app/flyout_window.cpp @@ -276,28 +276,30 @@ Flyout::Hit HitAt(const State& s, LPARAM l) { // ---------------------------------------------------------------- applying changes // Writes the pending keys into the live ini (read-modify-write, atomic), so a hand edit or a -// Settings write made meanwhile is kept. A locked ini keeps the changes pending and retries. -void FlushPending(State& s) { +// Settings write made meanwhile is kept. A locked ini, or a failed write, keeps the changes pending +// and retries (bounded); true = written, false = still pending or dropped. +bool FlushPending(State& s) { if (s.flushArmed) { KillTimer(s.hwnd, kFlushTimerId); s.flushArmed = false; } - if (s.pending.empty()) return; - std::string text; - if (GetFileAttributesW(s.iniPath.c_str()) != INVALID_FILE_ATTRIBUTES && - !wind::ReadTextFileOk(s.iniPath, text)) { - if (++s.flushRetries <= kMaxFlushRetries && !s.closing) { - SetTimer(s.hwnd, kFlushTimerId, Flyout::WriteThrottle::kMinMs, nullptr); - s.flushArmed = true; - } else { - wind::Log(wind::LogLevel::Warn, "tray", "flyout: ini unreadable, %zu change(s) dropped", s.pending.size()); - s.pending.clear(); - } - return; + if (s.pending.empty()) return true; + DWORD err = 0; + const wind::IniUpdate r = wind::UpdateIniKeys(s.iniPath, s.pending); + if (r == wind::IniUpdate::Ok) { + s.pending.clear(); + s.flushRetries = 0; + s.thr.wrote(GetTickCount64()); + return true; } - for (const auto& kv : s.pending) text = wind::UpdateIniText(text, kv.first, kv.second); - if (!wind::WriteTextFileAtomic(s.iniPath, text)) - wind::Log(wind::LogLevel::Warn, "tray", "flyout: ini write failed (err=%lu)", GetLastError()); - s.pending.clear(); - s.flushRetries = 0; - s.thr.wrote(GetTickCount64()); + if (r == wind::IniUpdate::WriteFailed) err = GetLastError(); + if (++s.flushRetries <= kMaxFlushRetries && !s.closing) { + SetTimer(s.hwnd, kFlushTimerId, Flyout::WriteThrottle::kMinMs, nullptr); + s.flushArmed = true; + } else { + wind::Log(wind::LogLevel::Warn, "tray", "flyout: ini %s (err=%lu), %zu change(s) dropped", + r == wind::IniUpdate::Unreadable ? "unreadable" : "write failed", err, s.pending.size()); + s.pending.clear(); + s.flushRetries = 0; + } + return false; } // Applies changes to the in-memory ini and the view at once; the file write is immediate when @@ -362,9 +364,12 @@ void ChooseEngine(int idx) { const std::wstring ini = g_f->iniPath; CloseList(); if (same) return; - FlushPending(*g_f); + // A slider write still pending (locked ini) rides along in the engine write instead of dying + // with the flyout (review item 87). + std::map carry; + if (!FlushPending(*g_f)) carry = g_f->pending; CloseFlyout(); - SetMainEngine(ini, idx); + SetMainEngine(ini, idx, carry); } void ChooseProfile(int idx) { diff --git a/src/tray_app/tray_app.h b/src/tray_app/tray_app.h index f7199bf0..6f8cc55e 100644 --- a/src/tray_app/tray_app.h +++ b/src/tray_app/tray_app.h @@ -4,6 +4,7 @@ // starts it with its PID; the helper exits when that process does. See // docs/specs/2026-09-29-tray-process-design.md. #include +#include #include namespace wind { @@ -35,7 +36,9 @@ int RunFlyoutTest(); // engine_dropdown.cpp - the flyout's main-engine dropdown (#315): writes `model` to the live ini and // restarts the Wind core (no prompt), keeping the unsaved session like Settings does. False = nothing // changed (same engine, or the write or relaunch failed, which it reports itself). -bool SetMainEngine(const std::wstring& ini, int picked); +// `carry`: ini keys the flyout still held unwritten; they are written in the same locked update. +bool SetMainEngine(const std::wstring& ini, int picked, + const std::map& carry = {}); // tray_menu.cpp - what the flyout's buttons do (no UI of their own). int UsesPalette(const std::string& iniText); // Wind's uiPalette as an index into Flyout::kPalettes; unknown or absent = 0 (Wind grey) diff --git a/src/tray_app/tray_menu.cpp b/src/tray_app/tray_menu.cpp index 25c549ba..a8007a76 100644 --- a/src/tray_app/tray_menu.cpp +++ b/src/tray_app/tray_menu.cpp @@ -70,6 +70,7 @@ static bool ConfirmUnsaved(const std::wstring& ini, bool forSwitch) { // The dialog blocks for as long as the user takes; Settings, the flyout or a hand edit may have // written the ini meanwhile. Act on the CURRENT text, not the snapshot the prompt was built from // (Save would store a stale session, Discard would overwrite newer globals such as the theme). + wind::IniWriteLock iniLock; // read-modify-write of the live ini (review item 71) if (!wind::ReadLiveIni(ini, live)) { Notify(L"Wind", L"Could not read the settings (config file is locked); nothing was changed."); return false; @@ -120,6 +121,7 @@ void SwitchToProfile(const std::wstring& ini, const std::wstring& nameW) { Notify(L"Wind", L"That profile's file looks corrupt; settings unchanged."); return; } } + wind::IniWriteLock iniLock; // held to the end, rollback included (review item 71) std::string oldLive; if (!wind::ReadLiveIni(ini, oldLive)) { wind::Log(wind::LogLevel::Warn, "profile", "switch aborted: live ini unreadable"); diff --git a/src/tray_ipc.h b/src/tray_ipc.h index 74297086..80db2e2c 100644 --- a/src/tray_ipc.h +++ b/src/tray_ipc.h @@ -50,6 +50,8 @@ inline void InitTrayBlock(TrayShared& b, uint32_t pid) { b.level.store(1.0, std::memory_order_relaxed); b.engine.store(0, std::memory_order_relaxed); b.panning.store(0, std::memory_order_relaxed); + b.menuOpen.store(0, std::memory_order_relaxed); // a leftover block may carry the last tray's "open" + b.ticks.reset(); // ...and the last Wind's pacing samples (review item 89) b.version.store(TrayShared::kVersion, std::memory_order_relaxed); b.magic.store(TrayShared::kMagic, std::memory_order_release); } diff --git a/src/version.h b/src/version.h index f84bfdc8..e143dcb7 100644 --- a/src/version.h +++ b/src/version.h @@ -4,7 +4,7 @@ #define WIND_VER_MAJOR 0 #define WIND_VER_MINOR 24 -#define WIND_VER_PATCH 12 +#define WIND_VER_PATCH 13 // String form for logs/snapshot/UI. Keep in sync with the numeric parts above. -#define WIND_VERSION_STR "0.24.12" +#define WIND_VERSION_STR "0.24.13" diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 34c9e6a4..da17f351 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -686,3 +686,25 @@ TEST_CASE("txSamplingMode folds the nearest aliases 2..4 to 0 (review 2026-10-09 // Folded to 0, the MPO guard sees an MPO-enabled boot's nearest and swaps in smooth. CHECK(EffectiveSamplingMode(ParseConfig("txSamplingMode=3\n").txSamplingMode, false, false, 0) == 1); } + +TEST_CASE("ParseConfig rejects NaN, infinity, hex and trailing junk, and keeps the default (review 2026-10-09 #72)") { + CHECK(ParseConfig("maxLevel=nan\n").maxLevel == doctest::Approx(12.0)); + CHECK(ParseConfig("maxLevel=inf\n").maxLevel == doctest::Approx(12.0)); + CHECK(ParseConfig("maxLevel=0x10\n").maxLevel == doctest::Approx(12.0)); + CHECK(ParseConfig("maxLevel=8abc\n").maxLevel == doctest::Approx(12.0)); + CHECK(ParseConfig("txMaxStepPct=0x10\n").txMaxStepPct == 25); + CHECK(ParseConfig("txMaxStepPct=7junk\n").txMaxStepPct == 25); + CHECK(ParseConfig("maxLevel=8\n").maxLevel == doctest::Approx(8.0)); + CHECK(ParseConfig("maxLevel=8.5\n").maxLevel == doctest::Approx(8.5)); + CHECK(ParseConfig("txMaxStepPct=40\n").txMaxStepPct == 40); + CHECK(ParseConfig("txMaxStepPct=10.0\n").txMaxStepPct == 10); // a float written for an integer key still reads +} +TEST_CASE("ParseConfig strips a UTF-8 BOM so the first key is not lost (review #72)") { + CHECK(ParseConfig("\xEF\xBB\xBFmaxLevel=8\n").maxLevel == doctest::Approx(8.0)); +} +TEST_CASE("ParseConfig clamps trackGlideMs and txMaxStepPct (review #72)") { + CHECK(ParseConfig("trackGlideMs=-50\n").trackGlideMs == 0); + CHECK(ParseConfig("trackGlideMs=999999\n").trackGlideMs == 5000); + CHECK(ParseConfig("txMaxStepPct=-5\n").txMaxStepPct == 0); + CHECK(ParseConfig("txMaxStepPct=99999\n").txMaxStepPct == 1000); +} diff --git a/tests/test_profiles.cpp b/tests/test_profiles.cpp index a9e7c5d4..49cd878f 100644 --- a/tests/test_profiles.cpp +++ b/tests/test_profiles.cpp @@ -202,10 +202,6 @@ TEST_CASE("SessionDiffers: keys outside the template stay missing-vs-present") { CHECK(SessionDiffers("someFutureKey=0\n", "")); CHECK(SessionDiffers("", "someFutureKey=0\n")); } -TEST_CASE("SessionDiffers: model is never filled from the template default") { - // The core's missing-key engine and the template's explicit one are different questions. - CHECK(SessionDiffers("model=hybrid\n", "")); -} TEST_CASE("ParseIniTmpName recognises WriteTextFileAtomic's leftover temp names") { unsigned long pid = 0; CHECK(ParseIniTmpName(L"magnifier.ini.1234.tmp", pid)); CHECK(pid == 1234); @@ -232,3 +228,28 @@ TEST_CASE("StripUiOnlyKeys strips every global key except profile (tray edits mu // Prefix siblings are different keys. CHECK(StripUiOnlyKeys("traySlidersX=1\n") == "traySlidersX=1\n"); } + +TEST_CASE("profile names: a device name is reserved with any extension, and the cap counts characters (review 2026-10-09 #73, #90)") { + CHECK(ProfileNameError("con.x") != ""); + CHECK(ProfileNameError("NUL.txt") != ""); + CHECK(ProfileNameError("com1 .ini") != ""); + CHECK(ProfileNameError("Lpt9.a.b") != ""); + CHECK(ProfileNameError("console") == ""); // only the exact device stem is reserved + CHECK(ProfileNameError("con x") == ""); + // 40 two-byte characters (80 bytes) fit; 41 do not. + std::string accents; for (int i = 0; i < 40; ++i) accents += "\xC3\xA6"; + CHECK(ProfileNameError(accents) == ""); + CHECK(ProfileNameError(accents + "\xC3\xA6") != ""); +} + +TEST_CASE("SessionDiffers: UI keys outside the first-run template read as their built-in default (review #70, #76)") { + CHECK_FALSE(SessionDiffers("zoomEaseOutMs=45\n", "maxLevel=12\n")); + CHECK(SessionDiffers("zoomEaseOutMs=100\n", "maxLevel=12\n")); + CHECK_FALSE(SessionDiffers("txSamplingMode=0\n", "")); + CHECK(SessionDiffers("txSamplingMode=1\n", "")); + CHECK_FALSE(SessionDiffers("lockApps=\n", "")); + CHECK(SessionDiffers("lockApps=game.exe\n", "")); + // The core's missing-key engine is hybrid, the template's too: no special case for `model`. + CHECK_FALSE(SessionDiffers("model=hybrid\n", "")); + CHECK(SessionDiffers("model=render\n", "")); +} diff --git a/tests/test_telemetry.cpp b/tests/test_telemetry.cpp index a96f64ba..3cade38d 100644 --- a/tests/test_telemetry.cpp +++ b/tests/test_telemetry.cpp @@ -49,3 +49,11 @@ TEST_CASE("telemetry line formatting never overflows a small buffer") { char tiny[8]; CHECK(FormatTelemetryLine(tiny, (int)sizeof(tiny), s) == 0); } + +TEST_CASE("a stale testlog.txt control file is ignored (review 2026-10-09 #69)") { + CHECK(wind::TestlogControlFresh(0)); + CHECK(wind::TestlogControlFresh(3600)); + CHECK(wind::TestlogControlFresh(-5)); // clock skew counts as fresh + CHECK_FALSE(wind::TestlogControlFresh(24 * 3600)); + CHECK_FALSE(wind::TestlogControlFresh(30LL * 24 * 3600)); +} diff --git a/tests/test_tray_ipc.cpp b/tests/test_tray_ipc.cpp index 9a772fd8..59fa6f61 100644 --- a/tests/test_tray_ipc.cpp +++ b/tests/test_tray_ipc.cpp @@ -78,3 +78,16 @@ TEST_CASE("the tick ring inside the block works from zero-filled memory") { CHECK(out[0] == doctest::Approx(6.9f)); CHECK(out[1] == doctest::Approx(7.0f)); } + +TEST_CASE("InitTrayBlock clears a leftover pacing ring and menu flag (review 2026-10-09 #89)") { + auto b = Fresh(); + InitTrayBlock(*b, 1); + b->ticks.push(16.7f); + b->ticks.push(16.7f); + SetTrayMenuOpen(b.get(), true); + REQUIRE_FALSE(b->ticks.empty()); + InitTrayBlock(*b, 2); // a new Wind adopting the same mapping + CHECK(b->ticks.empty()); + CHECK_FALSE(TrayMenuOpen(b.get())); + CHECK(b->windPid.load() == 2u); +} diff --git a/ui/src/prefs/profileName.js b/ui/src/prefs/profileName.js index d25fe9c9..b6f3c08f 100644 --- a/ui/src/prefs/profileName.js +++ b/ui/src/prefs/profileName.js @@ -2,14 +2,16 @@ // profile is a file, so the name must be a legal Windows file name. Pure, no DOM, so tests import it. const forbidden = /[\\/:*?"<>|]/; const reserved = /^(con|prn|aux|nul|com[1-9]|lpt[1-9])$/i; +// Windows reserves a device name with any extension too ("con.x", "NUL.txt"), and ignores spaces before the dot. +const stemOf = (t) => t.split('.')[0].replace(/ +$/, ''); // '' when the name is fine, else the sentence to show under the field. `names` = the existing profiles. export function nameError(t, names = []) { if (!t.trim()) return 'Enter a name for the profile.'; - if (t.length > 40) return 'The name is too long (40 characters at most).'; + if ([...t].length > 40) return 'The name is too long (40 characters at most).'; if (forbidden.test(t) || [...t].some((c) => c.charCodeAt(0) < 32)) return 'The name contains a character a file name cannot have.'; if (t !== t.trim() || t.startsWith('.') || t.endsWith('.')) return 'The name cannot start or end with a space or a dot.'; - if (reserved.test(t)) return 'Windows reserves that name.'; + if (reserved.test(stemOf(t))) return 'Windows reserves that name.'; if (names.some((x) => x.toLowerCase() === t.toLowerCase())) return 'A profile with this name already exists.'; return ''; }