[PATCH 0/1] MR2609: shlwapi: Implement StrFormatByteSizeEx
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54733 Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2609
From: David Kahurani <k.kahurani(a)gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54733 Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/shlwapi/shlwapi.spec | 1 + dlls/shlwapi/string.c | 133 ++++++++++++++++++++++++++---------- dlls/shlwapi/tests/string.c | 32 +++++++++ include/shlwapi.h | 1 + include/shtypes.idl | 5 ++ 5 files changed, 135 insertions(+), 37 deletions(-) diff --git a/dlls/shlwapi/shlwapi.spec b/dlls/shlwapi/shlwapi.spec index 801b2c9a890..0d4729b432f 100644 --- a/dlls/shlwapi/shlwapi.spec +++ b/dlls/shlwapi/shlwapi.spec @@ -783,6 +783,7 @@ @ stdcall StrFormatByteSizeA(long ptr long) @ stdcall StrFormatByteSizeW(int64 ptr long) @ stdcall StrFormatKBSizeA(int64 str long) +@ stdcall StrFormatByteSizeEx(int64 long ptr long) @ stdcall StrFormatKBSizeW(int64 wstr long) @ stdcall StrFromTimeIntervalA(ptr long long long) @ stdcall StrFromTimeIntervalW(ptr long long long) diff --git a/dlls/shlwapi/string.c b/dlls/shlwapi/string.c index 77e3b978cee..4c12796c0c1 100644 --- a/dlls/shlwapi/string.c +++ b/dlls/shlwapi/string.c @@ -823,23 +823,7 @@ typedef struct tagSHLWAPI_BYTEFORMATS WCHAR wPrefix; } SHLWAPI_BYTEFORMATS; -/************************************************************************* - * StrFormatByteSizeW [SHLWAPI.@] - * - * Create a string containing an abbreviated byte count of up to 2^63-1. - * - * PARAMS - * llBytes [I] Byte size to format - * lpszDest [I] Destination for formatted string - * cchMax [I] Size of lpszDest - * - * RETURNS - * lpszDest. - * - * NOTES - * There is no StrFormatByteSize64W function, it is called StrFormatByteSizeW(). - */ -LPWSTR WINAPI StrFormatByteSizeW(LONGLONG llBytes, LPWSTR lpszDest, UINT cchMax) +static double PrepareDouble(LONGLONG llBytes, SHLWAPI_BYTEFORMATS *bfFormat) { #define KB ((ULONGLONG)1024) #define MB (KB*KB) @@ -866,23 +850,8 @@ LPWSTR WINAPI StrFormatByteSizeW(LONGLONG llBytes, LPWSTR lpszDest, UINT cchMax) { 1000*PB, 1073741824.0, 1.00, 0, 'P' }, /* 1000 PB */ { 0, 10995116277.76, 100.00, 2, 'E' } /* EB's, catch all */ }; - WCHAR wszAdd[] = {' ','?','B',0}; - double dBytes; UINT i = 0; - TRACE("(0x%s,%p,%d)\n", wine_dbgstr_longlong(llBytes), lpszDest, cchMax); - - if (!lpszDest || !cchMax) - return lpszDest; - - if (llBytes < 1024) /* 1K */ - { - WCHAR wszBytesFormat[64]; - LoadStringW(shlwapi_hInstance, IDS_BYTES_FORMAT, wszBytesFormat, 64); - swprintf(lpszDest, cchMax, wszBytesFormat, (int)llBytes); - return lpszDest; - } - /* Note that if this loop completes without finding a match, i will be * pointing at the last entry, which is a catch all for > 1000 PB */ @@ -892,6 +861,8 @@ LPWSTR WINAPI StrFormatByteSizeW(LONGLONG llBytes, LPWSTR lpszDest, UINT cchMax) break; i++; } + + *bfFormat = bfFormats[i]; /* Above 1 TB we encounter problems with FP accuracy. So for amounts above * this number we integer shift down by 1 MB first. The table above has * the divisors scaled down from the '< 10 TB' entry onwards, to account @@ -899,15 +870,52 @@ LPWSTR WINAPI StrFormatByteSizeW(LONGLONG llBytes, LPWSTR lpszDest, UINT cchMax) * counts that lie exactly on a 1024 byte boundary. */ if (i > 8) - dBytes = (double)(llBytes >> 20) + 0.001; /* Scale down by 1 MB */ + return (double)(llBytes >> 20) + 0.001; /* Scale down by 1 MB */ else - dBytes = (double)llBytes + 0.00001; + return (double)llBytes + 0.00001; +} + +/************************************************************************* + * StrFormatByteSizeW [SHLWAPI.@] + * + * Create a string containing an abbreviated byte count of up to 2^63-1. + * + * PARAMS + * llBytes [I] Byte size to format + * lpszDest [I] Destination for formatted string + * cchMax [I] Size of lpszDest + * + * RETURNS + * lpszDest. + * + * NOTES + * There is no StrFormatByteSize64W function, it is called StrFormatByteSizeW(). + */ +LPWSTR WINAPI StrFormatByteSizeW(LONGLONG llBytes, LPWSTR lpszDest, UINT cchMax) +{ + WCHAR wszAdd[] = {' ','?','B',0}; + double dBytes; + SHLWAPI_BYTEFORMATS bfFormat; - dBytes = floor(dBytes / bfFormats[i].dDivisor) / bfFormats[i].dNormaliser; + TRACE("(0x%s,%p,%d)\n", wine_dbgstr_longlong(llBytes), lpszDest, cchMax); - if (!FormatDouble(dBytes, bfFormats[i].nDecimals, lpszDest, cchMax)) + if (!lpszDest || !cchMax) + return lpszDest; + + if (llBytes < 1024) /* 1K */ + { + WCHAR wszBytesFormat[64]; + LoadStringW(shlwapi_hInstance, IDS_BYTES_FORMAT, wszBytesFormat, 64); + swprintf(lpszDest, cchMax, wszBytesFormat, (int)llBytes); + return lpszDest; + } + + dBytes = PrepareDouble(llBytes, &bfFormat); + dBytes = floor(dBytes / bfFormat.dDivisor) / bfFormat.dNormaliser; + + if (!FormatDouble(dBytes, bfFormat.nDecimals, lpszDest, cchMax)) return NULL; - wszAdd[1] = bfFormats[i].wPrefix; + wszAdd[1] = bfFormat.wPrefix; StrCatBuffW(lpszDest, wszAdd, cchMax); return lpszDest; } @@ -952,6 +960,57 @@ LPSTR WINAPI StrFormatByteSizeA(DWORD dwBytes, LPSTR lpszDest, UINT cchMax) return StrFormatByteSize64A(dwBytes, lpszDest, cchMax); } +/************************************************************************* + * StrFormatByteSizeEx [SHLWAPI.@] + * + * Create a string containing an abbreviated by count with rounding options + * + * PARAMS + * dwBytes [I] Byte size to format + * flags [I] Enumeration values specifying whether to round or truncate + * lpszDest [I] Destination for the formatted string + * cchMax [I] Size of lpszDest + * + * RETURNS + * S_OK if conversion is successful otherwise returns a HRESULT error code. + * + */ + +HRESULT WINAPI StrFormatByteSizeEx(LONGLONG llBytes, SFBS_FLAGS flags, LPWSTR lpszDest, + UINT cchMax) +{ + WCHAR wszAdd[] = {' ','?','B',0}; + double dBytes; + SHLWAPI_BYTEFORMATS bfFormat; + + TRACE("(0x%s,%d,%p,%d)\n", wine_dbgstr_longlong(llBytes), flags, lpszDest, cchMax); + + if (!cchMax || (flags < 1 || flags > 2)) + return E_INVALIDARG; + + if (llBytes < 1024) /* 1K */ + { + WCHAR wszBytesFormat[64]; + LoadStringW(shlwapi_hInstance, IDS_BYTES_FORMAT, wszBytesFormat, 64); + swprintf(lpszDest, cchMax, wszBytesFormat, (int)llBytes); + return S_OK; + } + + dBytes = PrepareDouble(llBytes, &bfFormat); + + if (flags & SFBS_FLAGS_ROUND_TO_NEAREST_DISPLAYED_DIGIT) + dBytes = round(dBytes / bfFormat.dDivisor) / bfFormat.dNormaliser; + else if (flags & SFBS_FLAGS_TRUNCATE_UNDISPLAYED_DECIMAL_DIGITS) + dBytes = floor(dBytes / bfFormat.dDivisor) / bfFormat.dNormaliser; + + /* Not sure about the error code. Can't find a way to trigger the error here */ + if (!FormatDouble(dBytes, bfFormat.nDecimals, lpszDest, cchMax)) + return E_UNEXPECTED; + wszAdd[1] = bfFormat.wPrefix; + StrCatBuffW(lpszDest, wszAdd, cchMax); + return S_OK; +} + /************************************************************************* * @ [SHLWAPI.203] * diff --git a/dlls/shlwapi/tests/string.c b/dlls/shlwapi/tests/string.c index d988e26acb4..0c643a02797 100644 --- a/dlls/shlwapi/tests/string.c +++ b/dlls/shlwapi/tests/string.c @@ -52,6 +52,7 @@ static DWORD (WINAPI *pStrCatChainW)(LPWSTR,DWORD,DWORD,LPCWSTR); static LPSTR (WINAPI *pStrCpyNXA)(LPSTR,LPCSTR,int); static LPWSTR (WINAPI *pStrCpyNXW)(LPWSTR,LPCWSTR,int); static LPSTR (WINAPI *pStrFormatByteSize64A)(LONGLONG,LPSTR,UINT); +static HRESULT (WINAPI *pStrFormatByteSizeEx)(LONGLONG,SFBS_FLAGS,LPWSTR,UINT); static LPSTR (WINAPI *pStrFormatKBSizeA)(LONGLONG,LPSTR,UINT); static LPWSTR (WINAPI *pStrFormatKBSizeW)(LONGLONG,LPWSTR,UINT); static BOOL (WINAPI *pStrIsIntlEqualA)(BOOL,LPCSTR,LPCSTR,int); @@ -715,6 +716,35 @@ static void test_StrFormatByteSize64A(void) } } +static void test_StrFormatByteSizeEx(void) +{ + WCHAR szBuff[256]; + HRESULT hr; + LONGLONG test_value = 2147483647; + + if (!pStrFormatByteSizeEx) + { + win_skip("StrFormatByteSizeEx is not available \n"); + return; + } + + hr = pStrFormatByteSizeEx(0xdeadbeef, 2, szBuff, 0); + ok(hr == E_INVALIDARG, "Error code : %#lx expected : %#lx\n", hr, E_INVALIDARG); + + hr = pStrFormatByteSizeEx(0xdeadbeef, 10, szBuff, 256); + ok(hr == E_INVALIDARG, "Error code : %#lx , expected : %#lx\n", hr, E_INVALIDARG); + + hr = pStrFormatByteSizeEx(2147483647, 1, szBuff, 256); + ok(hr == S_OK, "Invalid arguments \n"); + ok(!wcscmp(szBuff, L"2.00 GB"), "Formatted %s wrong: got %ls, expected 2.00 GB\n", + wine_dbgstr_longlong(test_value), szBuff); + + hr = pStrFormatByteSizeEx(2147483647, 2, szBuff, 256); + ok(hr == S_OK, "Invalid arguments \n"); + ok(!wcscmp(szBuff, L"1.99 GB"), "Formatted %s wrong: got %ls, expected 1.99 GB\n", + wine_dbgstr_longlong(test_value), szBuff); +} + static void test_StrFormatKBSizeW(void) { WCHAR szBuffW[256]; @@ -1680,6 +1710,7 @@ START_TEST(string) pStrCpyNXW = (void *)GetProcAddress(hShlwapi, (LPSTR)400); pStrChrNW = (void *)GetProcAddress(hShlwapi, "StrChrNW"); pStrFormatByteSize64A = (void *)GetProcAddress(hShlwapi, "StrFormatByteSize64A"); + pStrFormatByteSizeEx = (void*)GetProcAddress(hShlwapi, "StrFormatByteSizeEx"); pStrFormatKBSizeA = (void *)GetProcAddress(hShlwapi, "StrFormatKBSizeA"); pStrFormatKBSizeW = (void *)GetProcAddress(hShlwapi, "StrFormatKBSizeW"); pStrIsIntlEqualA = (void *)GetProcAddress(hShlwapi, "StrIsIntlEqualA"); @@ -1716,6 +1747,7 @@ START_TEST(string) if (is_lang_english() && is_locale_english()) { test_StrFormatByteSize64A(); + test_StrFormatByteSizeEx(); test_StrFormatKBSizeA(); test_StrFormatKBSizeW(); } diff --git a/include/shlwapi.h b/include/shlwapi.h index eee41abf0dc..4f9803e0e0f 100644 --- a/include/shlwapi.h +++ b/include/shlwapi.h @@ -855,6 +855,7 @@ WINSHLWAPI HRESULT WINAPI SHStrDupW(LPCWSTR,WCHAR**); #define SHStrDup WINELIB_NAME_AW(SHStrDup) WINSHLWAPI LPSTR WINAPI StrFormatByteSizeA (DWORD,LPSTR,UINT); +WINSHLWAPI HRESULT WINAPI StrFormatByteSizeEx(LONGLONG,SFBS_FLAGS,LPWSTR,UINT); /* A/W Pairing is broken for this function */ WINSHLWAPI LPSTR WINAPI StrFormatByteSize64A (LONGLONG,LPSTR,UINT); diff --git a/include/shtypes.idl b/include/shtypes.idl index c83562a24bb..1b6e7c78e90 100644 --- a/include/shtypes.idl +++ b/include/shtypes.idl @@ -182,3 +182,8 @@ typedef [v1_enum] enum DEVICE_SCALE_FACTOR SCALE_450_PERCENT = 450, SCALE_500_PERCENT = 500 } DEVICE_SCALE_FACTOR; + +typedef [v1_enum] enum tagSFBS_FLAGS { + SFBS_FLAGS_ROUND_TO_NEAREST_DISPLAYED_DIGIT = 0x0001, + SFBS_FLAGS_TRUNCATE_UNDISPLAYED_DECIMAL_DIGITS = 0x0002 +} SFBS_FLAGS; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2609
Nikolay Sivov (@nsivov) commented about dlls/shlwapi/tests/string.c:
+ + hr = pStrFormatByteSizeEx(0xdeadbeef, 2, szBuff, 0); + ok(hr == E_INVALIDARG, "Error code : %#lx expected : %#lx\n", hr, E_INVALIDARG); + + hr = pStrFormatByteSizeEx(0xdeadbeef, 10, szBuff, 256); + ok(hr == E_INVALIDARG, "Error code : %#lx , expected : %#lx\n", hr, E_INVALIDARG); + + hr = pStrFormatByteSizeEx(2147483647, 1, szBuff, 256); + ok(hr == S_OK, "Invalid arguments \n"); + ok(!wcscmp(szBuff, L"2.00 GB"), "Formatted %s wrong: got %ls, expected 2.00 GB\n", + wine_dbgstr_longlong(test_value), szBuff); + + hr = pStrFormatByteSizeEx(2147483647, 2, szBuff, 256); + ok(hr == S_OK, "Invalid arguments \n"); + ok(!wcscmp(szBuff, L"1.99 GB"), "Formatted %s wrong: got %ls, expected 1.99 GB\n", + wine_dbgstr_longlong(test_value), szBuff); Here you might as well use enum member names for flags. For ok() traces, imho simply calling it "Unexpected" and printing actual value is enough.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2609#note_29296
Nikolay Sivov (@nsivov) commented about dlls/shlwapi/string.c:
- if (!FormatDouble(dBytes, bfFormats[i].nDecimals, lpszDest, cchMax)) + if (!lpszDest || !cchMax) + return lpszDest; + + if (llBytes < 1024) /* 1K */ + { + WCHAR wszBytesFormat[64]; + LoadStringW(shlwapi_hInstance, IDS_BYTES_FORMAT, wszBytesFormat, 64); + swprintf(lpszDest, cchMax, wszBytesFormat, (int)llBytes); + return lpszDest; + } + + dBytes = PrepareDouble(llBytes, &bfFormat); + dBytes = floor(dBytes / bfFormat.dDivisor) / bfFormat.dNormaliser; + + if (!FormatDouble(dBytes, bfFormat.nDecimals, lpszDest, cchMax)) Does it work if you simply call Ex() function here with TRUNCATE flag?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2609#note_29297
Nikolay Sivov (@nsivov) commented about dlls/shlwapi/string.c:
+/************************************************************************* + * StrFormatByteSizeEx [SHLWAPI.@] + * + * Create a string containing an abbreviated by count with rounding options + * + * PARAMS + * dwBytes [I] Byte size to format + * flags [I] Enumeration values specifying whether to round or truncate + * lpszDest [I] Destination for the formatted string + * cchMax [I] Size of lpszDest + * + * RETURNS + * S_OK if conversion is successful otherwise returns a HRESULT error code. + * + */
Just the function name is fine for the header. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2609#note_29298
Nikolay Sivov (@nsivov) commented about dlls/shlwapi/string.c:
+ * RETURNS + * S_OK if conversion is successful otherwise returns a HRESULT error code. + * + */ + +HRESULT WINAPI StrFormatByteSizeEx(LONGLONG llBytes, SFBS_FLAGS flags, LPWSTR lpszDest, + UINT cchMax) +{ + WCHAR wszAdd[] = {' ','?','B',0}; + double dBytes; + SHLWAPI_BYTEFORMATS bfFormat; + + TRACE("(0x%s,%d,%p,%d)\n", wine_dbgstr_longlong(llBytes), flags, lpszDest, cchMax); + + if (!cchMax || (flags < 1 || flags > 2)) + return E_INVALIDARG; Flag checks like that are not very readable. Does flags == 0 work?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2609#note_29299
On Fri Apr 7 12:19:55 2023 +0000, Nikolay Sivov wrote:
Does it work if you simply call Ex() function here with TRUNCATE flag? I didn't think about going about it this way. It should work except that Ex() function doesn't seem to validate the buffer. It throws an exception on a NULL buffer while the relative doesn't.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2609#note_29401
On Sat Apr 8 07:04:57 2023 +0000, David Kahurani wrote:
I didn't think about going about it this way. It should work except that Ex() function doesn't seem to validate the buffer. It throws an exception on a NULL buffer while the relative doesn't. I guess we can keep that check and use the Ex() function for everything else. We might have to manually check for specific error codes, though.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2609#note_29402
On Fri Apr 7 12:19:55 2023 +0000, Nikolay Sivov wrote:
Flag checks like that are not very readable. Does flags == 0 work? No, it does not. `flags == 0` results in error code too
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2609#note_29403
On Sat Apr 8 07:32:26 2023 +0000, David Kahurani wrote:
No, it does not. `flags == 0` results in error code too I guess we could use a switch and default to an error but then again that might seem like an overkill solution?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2609#note_29404
participants (3)
-
David Kahurani -
David Kahurani (＠kahurani)
-
Nikolay Sivov (＠nsivov)