https://git.reactos.org/?p=reactos.git;a=commitdiff;h=8ec05f5fa1108a50f378e57b76b5015ab7613ffe
commit 8ec05f5fa1108a50f378e57b76b5015ab7613ffe Author: Bișoc George <[email protected]> AuthorDate: Thu Dec 12 00:24:20 2019 +0100 Commit: Katayama Hirofumi MZ <[email protected]> CommitDate: Thu Dec 12 08:24:20 2019 +0900 [SHELL32] Do not return -1 if a file is not valid or not found - ExtractIconEx() (#2113) ExtractIconEx() returns the number of successfully extracted icons from a file. The routine may return 0 in case no icons could be extracted but it could also return 0 if the file is not a valid PE image file or the file couldn't be found. PrivateExtractIcons and the internal USER32 routine, ICO_ExtractIconExW(), return -1 in such scenarios. The behaviour is correct however we do not want that ExtractIconEx() returns -1 as well as it doesn't comply with the general documentation. In such cases, simply return 0 as no successful icons have been extracted due to related file failures. CORE-16535 --- dll/win32/shell32/iconcache.cpp | 20 +++++++++--- modules/rostests/apitests/shell32/CMakeLists.txt | 1 + .../rostests/apitests/shell32/ExtractIconEx.cpp | 37 ++++++++++++++++++++++ modules/rostests/apitests/shell32/testlist.c | 2 ++ 4 files changed, 56 insertions(+), 4 deletions(-) diff --git a/dll/win32/shell32/iconcache.cpp b/dll/win32/shell32/iconcache.cpp index 400f75cc456..5b35326a2b7 100644 --- a/dll/win32/shell32/iconcache.cpp +++ b/dll/win32/shell32/iconcache.cpp @@ -866,12 +866,13 @@ EXTERN_C INT WINAPI Shell_GetCachedImageIndex(LPCWSTR szPath, INT nIndex, UINT b /************************************************************************* * ExtractIconExW [SHELL32.@] * RETURNS - * 0 no icon found - * -1 file is not valid + * 0 no icon found (or the file is not valid) * or number of icons extracted */ UINT WINAPI ExtractIconExW(LPCWSTR lpszFile, INT nIconIndex, HICON * phiconLarge, HICON * phiconSmall, UINT nIcons) { + UINT ret = 0; + /* get entry point of undocumented function PrivateExtractIconExW() in user32 */ #if defined(__CYGWIN__) || defined (__MINGW32__) || defined(_MSC_VER) static UINT (WINAPI*PrivateExtractIconExW)(LPCWSTR,int,HICON*,HICON*,UINT) = NULL; @@ -881,13 +882,24 @@ UINT WINAPI ExtractIconExW(LPCWSTR lpszFile, INT nIconIndex, HICON * phiconLarge PrivateExtractIconExW = (UINT(WINAPI*)(LPCWSTR,int,HICON*,HICON*,UINT)) GetProcAddress(hUser32, "PrivateExtractIconExW"); if (!PrivateExtractIconExW) - return 0; + return ret; } #endif TRACE("%s %i %p %p %i\n", debugstr_w(lpszFile), nIconIndex, phiconLarge, phiconSmall, nIcons); + ret = PrivateExtractIconExW(lpszFile, nIconIndex, phiconLarge, phiconSmall, nIcons); - return PrivateExtractIconExW(lpszFile, nIconIndex, phiconLarge, phiconSmall, nIcons); + /* PrivateExtractIconExW() may return -1 if the provided file is not a valid PE image file or the said + * file couldn't be found. The behaviour is correct although ExtractIconExW() only returns the successfully + * extracted icons from a file. In such scenario, simply return 0. + */ + if (ret == 0xFFFFFFFF) + { + WARN("Invalid file or couldn't be found - %s\n", debugstr_w(lpszFile)); + ret = 0; + } + + return ret; } /************************************************************************* diff --git a/modules/rostests/apitests/shell32/CMakeLists.txt b/modules/rostests/apitests/shell32/CMakeLists.txt index f1cd11fefaa..22dbadf9dfb 100644 --- a/modules/rostests/apitests/shell32/CMakeLists.txt +++ b/modules/rostests/apitests/shell32/CMakeLists.txt @@ -18,6 +18,7 @@ list(APPEND SOURCE CUserNotification.cpp Control_RunDLLW.cpp DragDrop.cpp + ExtractIconEx.cpp IShellFolderViewCB.cpp OpenAs_RunDLL.cpp PathResolve.cpp diff --git a/modules/rostests/apitests/shell32/ExtractIconEx.cpp b/modules/rostests/apitests/shell32/ExtractIconEx.cpp new file mode 100644 index 00000000000..721c749350b --- /dev/null +++ b/modules/rostests/apitests/shell32/ExtractIconEx.cpp @@ -0,0 +1,37 @@ +/* + * PROJECT: ReactOS API Tests + * LICENSE: GPL-2.0-or-later (https://spdx.org/licenses/GPL-2.0-or-later) + * PURPOSE: Tests for ExtractIconEx routine + * COPYRIGHT: Copyright 2019 Bișoc George (fraizeraust99 at gmail dot com) + */ + +#include "shelltest.h" + +typedef struct +{ + PCWSTR pszFilePath; + UINT nIcons; +} EXTRACTICONTESTS; + +EXTRACTICONTESTS IconTests[] = +{ + /* Executable file with icon */ + {L"%SystemRoot%\\System32\\cmd.exe", 1}, + + /* Executable file without icon */ + {L"%SystemRoot%\\System32\\autochk.exe", 0}, + + /* Non-existing files */ + {L"%SystemRoot%\\non-existent-file.sdf", 0} +}; + +START_TEST(ExtractIconEx) +{ + UINT i, nReturnedIcons; + + for (i = 0; i < _countof(IconTests); ++i) + { + nReturnedIcons = ExtractIconExW(IconTests[i].pszFilePath, 0, NULL, NULL, IconTests[i].nIcons); + ok(nReturnedIcons == IconTests[i].nIcons, "ExtractIconExW(%u): Expected %u icons, got %u\n", i, IconTests[i].nIcons, nReturnedIcons); + } +} diff --git a/modules/rostests/apitests/shell32/testlist.c b/modules/rostests/apitests/shell32/testlist.c index a274688b0f5..18939f998e3 100644 --- a/modules/rostests/apitests/shell32/testlist.c +++ b/modules/rostests/apitests/shell32/testlist.c @@ -12,6 +12,7 @@ extern void func_CShellDesktop(void); extern void func_CShellLink(void); extern void func_CUserNotification(void); extern void func_DragDrop(void); +extern void func_ExtractIconEx(void); extern void func_IShellFolderViewCB(void); extern void func_menu(void); extern void func_OpenAs_RunDLL(void); @@ -35,6 +36,7 @@ const struct test winetest_testlist[] = { "CShellLink", func_CShellLink }, { "CUserNotification", func_CUserNotification }, { "DragDrop", func_DragDrop }, + { "ExtractIconEx", func_ExtractIconEx }, { "IShellFolderViewCB", func_IShellFolderViewCB }, { "menu", func_menu }, { "OpenAs_RunDLL", func_OpenAs_RunDLL },
