[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

@@ -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;
 }
 
 /*************************************************************************

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

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);
+    }
+}

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 },