[hbelusca] 74916: [SETUPAPI]: Fix a 9-year old bug in the SETUP_CreateClassKey() logic, introduced in r31860, that set a wrong Class name value if the corresponding registry key already existed. Th...

Author: hbelusca
Date: Sun Jun  4 18:16:24 2017
New Revision: 74916

URL: http://svn.reactos.org/svn/reactos?rev=74916&view=rev
Log:
[SETUPAPI]: Fix a 9-year old bug in the SETUP_CreateClassKey() logic, introduced in
r31860, that set a wrong Class name value if the corresponding registry key already
existed. The bug was unveiled by my commit r74761.
[SETUPAPI_APITEST]: When selecting another test_class_guid and test_class_name couple via
SetupDiClassNameFromGuidA(), check whether this function actually succeeds. Also, improve
some error output.

Modified:
    trunk/reactos/dll/win32/setupapi/devinst.c
    trunk/rostests/apitests/setupapi/devclass.c

Modified: trunk/reactos/dll/win32/setupapi/devinst.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/setupapi/devinst…
==============================================================================
--- trunk/reactos/dll/win32/setupapi/devinst.c	[iso-8859-1] (original)
+++ trunk/reactos/dll/win32/setupapi/devinst.c	[iso-8859-1] Sun Jun  4 18:16:24 2017
@@ -3599,58 +3599,72 @@
 
 HKEY SETUP_CreateClassKey(HINF hInf)
 {
-    static const WCHAR slash[] = { '\\',0 };
     WCHAR FullBuffer[MAX_PATH];
     WCHAR Buffer[MAX_PATH];
     DWORD RequiredSize;
     HKEY hClassKey;
-
+    DWORD Disposition;
+
+    /* Obtain the Class GUID for this class */
     if (!SetupGetLineTextW(NULL,
                            hInf,
                            Version,
                            REGSTR_VAL_CLASSGUID,
                            Buffer,
-                           MAX_PATH,
+                           sizeof(Buffer) / sizeof(WCHAR),
                            &RequiredSize))
     {
         return INVALID_HANDLE_VALUE;
     }
 
+    /* Build the corresponding registry key name */
     lstrcpyW(FullBuffer, REGSTR_PATH_CLASS_NT);
-    lstrcatW(FullBuffer, slash);
+    lstrcatW(FullBuffer, BackSlash);
     lstrcatW(FullBuffer, Buffer);
 
+    /* Obtain the Class name for this class */
+    if (!SetupGetLineTextW(NULL,
+                           hInf,
+                           Version,
+                           REGSTR_VAL_CLASS,
+                           Buffer,
+                           sizeof(Buffer) / sizeof(WCHAR),
+                           &RequiredSize))
+    {
+        return INVALID_HANDLE_VALUE;
+    }
+
+    /* Try to open or create the registry key */
+    TRACE("Opening class key %s\n", debugstr_w(FullBuffer));
+#if 0 // I keep this for reference...
     if (RegOpenKeyExW(HKEY_LOCAL_MACHINE,
                       FullBuffer,
                       0,
                       KEY_SET_VALUE,
                       &hClassKey))
     {
-        if (!SetupGetLineTextW(NULL,
-                               hInf,
-                               Version,
-                               REGSTR_VAL_CLASS,
-                               Buffer,
-                               MAX_PATH,
-                               &RequiredSize))
-        {
-            return INVALID_HANDLE_VALUE;
-        }
-
-        if (RegCreateKeyExW(HKEY_LOCAL_MACHINE,
-                            FullBuffer,
-                            0,
-                            NULL,
-                            REG_OPTION_NON_VOLATILE,
-                            KEY_SET_VALUE,
-                            NULL,
-                            &hClassKey,
-                            NULL))
-        {
-            return INVALID_HANDLE_VALUE;
-        }
-    }
-
+        /* Use RegCreateKeyExW */
+    }
+#endif
+    if (RegCreateKeyExW(HKEY_LOCAL_MACHINE,
+                        FullBuffer,
+                        0,
+                        NULL,
+                        REG_OPTION_NON_VOLATILE,
+                        KEY_SET_VALUE,
+                        NULL,
+                        &hClassKey,
+                        &Disposition))
+    {
+        ERR("RegCreateKeyExW(%s) failed\n", debugstr_w(FullBuffer));
+        return INVALID_HANDLE_VALUE;
+    }
+    if (Disposition == REG_CREATED_NEW_KEY)
+        TRACE("The class key %s was successfully created\n",
debugstr_w(FullBuffer));
+    else
+        TRACE("The class key %s was successfully opened\n",
debugstr_w(FullBuffer));
+
+    TRACE( "setting value %s to %s\n", debugstr_w(REGSTR_VAL_CLASS),
debugstr_w(Buffer) );
     if (RegSetValueExW(hClassKey,
                        REGSTR_VAL_CLASS,
                        0,

Modified: trunk/rostests/apitests/setupapi/devclass.c
URL:
http://svn.reactos.org/svn/reactos/trunk/rostests/apitests/setupapi/devclas…
==============================================================================
--- trunk/rostests/apitests/setupapi/devclass.c	[iso-8859-1] (original)
+++ trunk/rostests/apitests/setupapi/devclass.c	[iso-8859-1] Sun Jun  4 18:16:24 2017
@@ -89,9 +89,10 @@
 
     if ( size > 0 )
     {
-       /* That's better to use the first class found, as we know for sure that it
exists */
-       memcpy(&test_class_guid, &guid_list[0], sizeof( GUID ) );
-       SetupDiClassNameFromGuidA( &test_class_guid, test_class_name, sizeof(
test_class_name ), NULL );
+        /* That's better to use the first class found, as we know for sure that it
exists */
+        memcpy(&test_class_guid, &guid_list[0], sizeof( GUID ) );
+        ok( SetupDiClassNameFromGuidA( &test_class_guid, test_class_name, sizeof(
test_class_name ), NULL ),
+            "Error reported %lx\n", GetLastError() );
     }
     HeapFree( GetProcessHeap(), 0, guid_list );
 }
@@ -131,13 +132,13 @@
         "Error reported %lx\n", GetLastError() );
     ok( size == required_size, "Expected size %lu, got %lu\n", required_size,
size );
     ok( IsEqualIID( &guid_list[0], &test_class_guid ),
-        "Expected %s, got %s\n", debugstr_guid( &test_class_guid ),
debugstr_guid( &guid_list[0] ) );
+        "Expected %s, got %s for class %s\n", debugstr_guid(
&test_class_guid ), debugstr_guid( &guid_list[0] ), test_class_name );
     SetLastError( 0xdeadbeef );
     ok( SetupDiClassGuidsFromNameA( test_class_name, guid_list, required_size + 1,
&size ),
         "Error reported %lx\n", GetLastError() );
     ok( size == required_size, "Expected size %lu, got %lu\n", required_size,
size );
     ok( IsEqualIID( &guid_list[0], &test_class_guid ),
-        "Expected %s, got %s\n", debugstr_guid( &test_class_guid ),
debugstr_guid( &guid_list[0] ) );
+        "Expected %s, got %s for class %s\n", debugstr_guid(
&test_class_guid ), debugstr_guid( &guid_list[0] ), test_class_name );
 
     HeapFree( GetProcessHeap(), 0, guid_list );
 }
@@ -162,7 +163,7 @@
         "Fail expected\n" );
     ok_lasterr( ERROR_INSUFFICIENT_BUFFER );
     ok( required_size > 0, "Expected > 0, got %lu\n", required_size );
-    ok( required_size < MAX_CLASS_NAME_LEN, "Expected < %u, got %lu\n",
MAX_CLASS_NAME_LEN, required_size );
+    ok( required_size < MAX_CLASS_NAME_LEN, "Expected < %u, got %lu for GUID
%s\n", MAX_CLASS_NAME_LEN, required_size, debugstr_guid( &test_class_guid ) );
 
     class_name = HeapAlloc( GetProcessHeap(), 0, required_size );
     if ( !class_name )