[PATCH 0/2] MR5428: Fix substring operation in 'magic' variable expansion.
In substring expansion of variables, the length computation wasn't the same between: - magic variables: length doesn't include terminating NUL - regular variables: length does include terminating NUL This led to incorrect substring operation for magic variables. Always set length to not include terminating NUL. Took the opportunity to not recompute length (as it's already computed by called functions). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5428
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- programs/cmd/tests/test_builtins.cmd | 6 ++++++ programs/cmd/tests/test_builtins.cmd.exp | 2 ++ 2 files changed, 8 insertions(+) diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 1ab2935aec4..7517cca343f 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -622,7 +622,13 @@ echo %WINE_VAR:~2,-1% echo %WINE_VAR:~2,-3% echo '%WINE_VAR:~-2,-4%' echo %WINE_VAR:~-3,-2% +echo %WINE_VAR:~4,4% set WINE_VAR= +mkdir dummydir +cd dummydir +echo %CD:~-6,6% +cd .. +rmdir dummydir echo ------------ Testing variable substitution ------------ echo --- in FOR variables diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 0fb4a597ac5..7030748d549 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -532,6 +532,8 @@ ert(a)or_broken@qwerty e(a)or_broken@qwerty ''@or_broken@'qwerty' r(a)or_broken@qwerty +ty +(a)todo_wine@mmydir ------------ Testing variable substitution ------------ --- in FOR variables "A B" -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5428
From: Eric Pouech <epouech(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56498 Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- programs/cmd/tests/test_builtins.cmd.exp | 2 +- programs/cmd/wcmdmain.c | 32 ++++++++++-------------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 7030748d549..1365a5c3474 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -533,7 +533,7 @@ e(a)or_broken@qwerty ''@or_broken@'qwerty' r(a)or_broken@qwerty ty -(a)todo_wine@mmydir +mmydir ------------ Testing variable substitution ------------ --- in FOR variables "A B" diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 1867c5afb92..eb66b29836e 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -614,25 +614,19 @@ static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR startchar) /* Handle DATE, TIME, ERRORLEVEL and CD replacements allowing */ /* override if existing env var called that name */ if (WCMD_is_magic_envvar(thisVar, L"ERRORLEVEL")) { - wsprintfW(thisVarContents, L"%d", errorlevel); - len = lstrlenW(thisVarContents); + len = wsprintfW(thisVarContents, L"%d", errorlevel); } else if (WCMD_is_magic_envvar(thisVar, L"DATE")) { - GetDateFormatW(LOCALE_USER_DEFAULT, DATE_SHORTDATE, NULL, - NULL, thisVarContents, MAXSTRING); - len = lstrlenW(thisVarContents); + len = GetDateFormatW(LOCALE_USER_DEFAULT, DATE_SHORTDATE, NULL, + NULL, thisVarContents, ARRAY_SIZE(thisVarContents)); } else if (WCMD_is_magic_envvar(thisVar, L"TIME")) { - GetTimeFormatW(LOCALE_USER_DEFAULT, TIME_NOSECONDS, NULL, - NULL, thisVarContents, MAXSTRING); - len = lstrlenW(thisVarContents); + len = GetTimeFormatW(LOCALE_USER_DEFAULT, TIME_NOSECONDS, NULL, + NULL, thisVarContents, ARRAY_SIZE(thisVarContents)); } else if (WCMD_is_magic_envvar(thisVar, L"CD")) { - GetCurrentDirectoryW(MAXSTRING, thisVarContents); - len = lstrlenW(thisVarContents); + len = GetCurrentDirectoryW(ARRAY_SIZE(thisVarContents), thisVarContents); } else if (WCMD_is_magic_envvar(thisVar, L"RANDOM")) { - wsprintfW(thisVarContents, L"%d", rand() % 32768); - len = lstrlenW(thisVarContents); + len = wsprintfW(thisVarContents, L"%d", rand() % 32768); } else { - - len = ExpandEnvironmentStringsW(thisVar, thisVarContents, ARRAY_SIZE(thisVarContents)); + if ((len = ExpandEnvironmentStringsW(thisVar, thisVarContents, ARRAY_SIZE(thisVarContents)))) len--; } if (len == 0) @@ -704,9 +698,9 @@ static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR startchar) /* Check bounds */ if (substrposition >= 0) { - startCopy = &thisVarContents[min(substrposition, len)]; + startCopy = &thisVarContents[min(substrposition, len - 1)]; } else { - startCopy = &thisVarContents[max(0, len+substrposition-1)]; + startCopy = &thisVarContents[max(0, len + substrposition)]; } if (commapos == NULL) { @@ -714,12 +708,12 @@ static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR startchar) WCMD_strsubstW(start, endOfVar + 1, startCopy, -1); } else if (substrlength < 0) { - int copybytes = (len+substrlength-1)-(startCopy-thisVarContents); - if (copybytes > len) copybytes = len; + int copybytes = len + substrlength - (startCopy - thisVarContents); + if (copybytes >= len) copybytes = len - 1; else if (copybytes < 0) copybytes = 0; WCMD_strsubstW(start, endOfVar + 1, startCopy, copybytes); } else { - substrlength = min(substrlength, len - (startCopy- thisVarContents + 1)); + substrlength = min(substrlength, len - (startCopy - thisVarContents)); WCMD_strsubstW(start, endOfVar + 1, startCopy, substrlength); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5428
participants (2)
-
Eric Pouech -
eric pouech (＠epo)