comctl32: realloc or free and alloc

[Alexey Fisher]

Hallo,

i currently digg in comctl32 to find why my app fails. I found that
string conversation AtoW in some places silently fails. The problem is
that the destination for string is just a fresh pointer (not NULL).
Str_SetPtrAtoW check if it is NULL pointer and if not it trys to
ReAlloc. There was no Alloc before so ReAlloc returns NULL. Because
there is no other checks, Str_SetPtrAtoW just go back without converting
the string. 

Here is affected code:
dlls/comctl32/comctl32undoc.c:999
> BOOL Str_SetPtrAtoW (LPWSTR *lppDest, LPCSTR lpSrc)
> {
>     TRACE("(%p %s)\n", lppDest, lpSrc);
> 
>     if (lpSrc) {
>         INT len = MultiByteToWideChar(CP_ACP,0,lpSrc,-1,NULL,0);
>         LPWSTR ptr = ReAlloc (*lppDest, len*sizeof(WCHAR));
> 
>         if (!ptr) {
>             ERR("last error %x\n", GetLastError());
>             return FALSE;
>         }
>         MultiByteToWideChar(CP_ACP,0,lpSrc,-1,ptr,len);
>         *lppDest = ptr;
>     }
>     else {
>         Free (*lppDest);
>         *lppDest = NULL;
>     }
> 
>     return TRUE;
> }

The question is, is it appropriate reaction of ReAlloc and if it so, may be it make more sense to do this:

--- a/dlls/comctl32/comctl32undoc.c
+++ b/dlls/comctl32/comctl32undoc.c
@@ -998,18 +998,26 @@ INT Str_GetPtrAtoW (LPCSTR lpSrc, LPWSTR lpDest, INT nMaxLen)
  */
 BOOL Str_SetPtrAtoW (LPWSTR *lppDest, LPCSTR lpSrc)
 {
-    TRACE("(%p %s)\n", lppDest, lpSrc);
+    TRACE("(%p, %s)\n", *lppDest, debugstr_a(lpSrc));

     if (lpSrc) {
+        if (*lppDest) {
+            Free (*lppDest);
+            *lppDest = NULL;
+        }
        INT len = MultiByteToWideChar(CP_ACP,0,lpSrc,-1,NULL,0);
-       LPWSTR ptr = ReAlloc (*lppDest, len*sizeof(WCHAR));
+       LPWSTR ptr = Alloc (len*sizeof(WCHAR));

-       if (!ptr)
+       if (!ptr) {
+            ERR("Can't allocait memory size: %u,"
+                " for string %s.\n", len*sizeof(WCHAR), lpSrc);
            return FALSE;
+        }
        MultiByteToWideChar(CP_ACP,0,lpSrc,-1,ptr,len);
        *lppDest = ptr;
     }
     else {
+        TRACE("lpSrc = NULL, nothing to convert.\n");
         Free (*lppDest);
         *lppDest = NULL;
     }

-- 
Regards,
        Alexey