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