please help with patch submission

[Damjan Jovanovic]

Hi

I have a short patch that fixes serious bugs I've
encountered in setupapi's SetupDiOpenClassRegKeyExW().
It's been mailed twice to wine-patches and is attached
for reference. Is there anything wrong with it?

Please comment.

Thank you
Damjan


__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

--- wine-0.9.17-old/dlls/setupapi/devinst.c	2006-07-10 18:01:08.000000000 +0200
+++ wine-0.9.17/dlls/setupapi/devinst.c	2006-07-28 14:12:39.000000000 +0200
@@ -33,20 +33,21 @@
 #include "winnls.h"
 #include "setupapi.h"
 #include "wine/debug.h"
 #include "wine/unicode.h"
 #include "ddk/cfgmgr32.h"
 #include "initguid.h"
 #include "winioctl.h"
 #include "rpc.h"
 #include "rpcdce.h"
 
+#include "ddk/cfgmgr32.h"
 #include "setupapi_private.h"
 
 
 WINE_DEFAULT_DEBUG_CHANNEL(setupapi);
 
 /* Unicode constants */
 static const WCHAR ClassGUID[]  = {'C','l','a','s','s','G','U','I','D',0};
 static const WCHAR Class[]  = {'C','l','a','s','s',0};
 static const WCHAR ClassInstall32[]  = {'C','l','a','s','s','I','n','s','t','a','l','l','3','2',0};
 static const WCHAR NoDisplayClass[]  = {'N','o','D','i','s','p','l','a','y','C','l','a','s','s',0};
@@ -1314,20 +1315,21 @@
  *		SetupDiOpenClassRegKeyExW  (SETUPAPI.@)
  */
 HKEY WINAPI SetupDiOpenClassRegKeyExW(
         const GUID* ClassGuid,
         REGSAM samDesired,
         DWORD Flags,
         PCWSTR MachineName,
         PVOID Reserved)
 {
     LPWSTR lpGuidString;
+    WCHAR guidStringWithBraces[MAX_GUID_STRING_LEN];
     HKEY hClassesKey;
     HKEY hClassKey;
     LPCWSTR lpKeyName;
 
     if (MachineName != NULL)
     {
         FIXME("Remote access not supported yet!\n");
         return INVALID_HANDLE_VALUE;
     }
 
@@ -1354,35 +1356,39 @@
     {
 	return INVALID_HANDLE_VALUE;
     }
 
     if (ClassGuid == NULL)
         return hClassesKey;
 
     if (UuidToStringW((UUID*)ClassGuid, &lpGuidString) != RPC_S_OK)
     {
 	RegCloseKey(hClassesKey);
-	return FALSE;
+	return INVALID_HANDLE_VALUE;
     }
+    guidStringWithBraces[0] = (WCHAR) '{';
+    CopyMemory(&guidStringWithBraces[1], lpGuidString,
+        (MAX_GUID_STRING_LEN - 3) * sizeof(WCHAR));
+    guidStringWithBraces[1 + strlenW(lpGuidString)] = (WCHAR) '}';
+    guidStringWithBraces[2 + strlenW(lpGuidString)] = 0;
+    RpcStringFreeW(&lpGuidString);
 
     if (RegOpenKeyExW(hClassesKey,
-		      lpGuidString,
+		      guidStringWithBraces,
 		      0,
 		      KEY_ALL_ACCESS,
 		      &hClassKey))
     {
-	RpcStringFreeW(&lpGuidString);
 	RegCloseKey(hClassesKey);
-	return FALSE;
+	return INVALID_HANDLE_VALUE;
     }
 
-    RpcStringFreeW(&lpGuidString);
     RegCloseKey(hClassesKey);
 
     return hClassKey;
 }
 
 /***********************************************************************
  *		SetupDiOpenDeviceInterfaceW (SETUPAPI.@)
  */
 BOOL WINAPI SetupDiOpenDeviceInterfaceW(
        HDEVINFO DeviceInfoSet,

[Neil Skrypuch]

On Monday, July 31, 2006 06:37, Damjan Jovanovic wrote:
> +    WCHAR guidStringWithBraces[MAX_GUID_STRING_LEN];

You need MAX_GUID_STRING_LEN + 1 to store the null too, if I'm not mistaken.
> +    CopyMemory(&guidStringWithBraces[1], lpGuidString,
> +        (MAX_GUID_STRING_LEN - 3) * sizeof(WCHAR));

Why MAX_GUID_STRING_LEN - 3? Wouldn't that miss the last 3 characters?

- Neil

[Damjan Jovanovic]

--- Neil Skrypuch <ns03ja(a)brocku.ca> wrote:
> On Monday, July 31, 2006 06:37, Damjan Jovanovic
> wrote:
> > +    WCHAR
> > guidStringWithBraces[MAX_GUID_STRING_LEN];
>
> You need MAX_GUID_STRING_LEN + 1 to store the null
> too, if I'm not mistaken.

0         1         2         3
0123456789012345678901234567890123456789
{xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx}\0

MAX_GUID_STRING_LEN is 39, so it includes space for
the terminating NULL.
> > +    CopyMemory(&guidStringWithBraces[1],
> > lpGuidString,
> > +        (MAX_GUID_STRING_LEN - 3) *
> > sizeof(WCHAR));
>
> Why MAX_GUID_STRING_LEN - 3? Wouldn't that miss the
> last 3 characters?

No, because without braces and the terminating NULL it
is MAX_GUID_STRING_LEN - 3.
> - Neil

Thanks anyway
Damjan


__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com

[Neil Skrypuch]

Clearly it was too early for me when I wrote that. I had counted the GUID 
length wrong (as 39 instead of 38), so the - 3 didn't make sense either.

- Neil