Error path issues with "gameux: Add implementation of IGameStatisticsMgr::RemoveGameStatistics."

[Joris Huizer]

Hello,

I noticed the committed patch "gameux: Add implementation of IGameStatisticsMgr::RemoveGameStatistics." (5cac9d2cb2c020802a56a5b1b28348316f1087ba)

The GAMEUX_getAppIdFromGDFPath() function now ends with:

+    HeapFree(GetProcessHeap(), 0, lpRegistryPath);
+
+    TRACE("found app id: %s, return: %#x\n", debugstr_w(lpApplicationId), hr);
+    return hr;

In most of the error paths, lpRegistryPath is not initialized, so it's pointing to garbage; I think just initializing to NULL should be sufficient;

Similarly, in that case, lpApplicationId is not initialized, so it contains garbage; Tracing will probably print some random stack bytes before hitting a zero byte.
I'm not completely sure what should be done about this issue; I think it should probably only be traced on success, perhaps tracing the error otherwise; Any ideas?

HTH,
Joris

Error path issues with "gameux: Add implementation of IGameStatisticsMgr::RemoveGameStatistics."

[Mariusz Pluciński]

W dniu 29.09.2010 17:15, Joris Huizer pisze:
> Hello,
>
> I noticed the committed patch "gameux: Add implementation of
> IGameStatisticsMgr::RemoveGameStatistics."
> (5cac9d2cb2c020802a56a5b1b28348316f1087ba)
>
> The GAMEUX_getAppIdFromGDFPath() function now ends with:
>
> +    HeapFree(GetProcessHeap(), 0, lpRegistryPath); + +
> TRACE("found app id: %s, return: %#x\n", debugstr_w(lpApplicationId),
> hr); +    return hr;
>
> In most of the error paths, lpRegistryPath is not initialized, so
> it's pointing to garbage; I think just initializing to NULL should be
> sufficient;
>
> Similarly, in that case, lpApplicationId is not initialized, so it
> contains garbage; Tracing will probably print some random stack bytes
> before hitting a zero byte. I'm not completely sure what should be
> done about this issue; I think it should probably only be traced on
> success, perhaps tracing the error otherwise; Any ideas?
>
> HTH, Joris
> Hi
> I'm author of this patch.
> You're definitely right, I moved code of this function from other place, 
> and forgot to initialize these variables properly. I'll fix that by 
> initializing these variables (I do not want to make TRACE conditional, 
> as it prints HRESULT value too). Anyway, thanks for your notification.

-- 
Best regards
Mariusz Pluciński