Re: [PATCH] uxtheme: fixed the todo blocks in the SetWindowTheme tests. [try 2]
On Tue, Oct 28, 2008 at 2:29 AM, Reece Dunn <msclrhd(a)googlemail.com> wrote:
Hi,
This fixes the todo blocks in the SetWindowTheme tests.
Changes: v2 -- initialise the hr variable so that it is initialised in the case where hwnd is a valid window handle.
Please fix your changelog entry to describe the *fix*, not the end result (getting rid of todo_wine's). Also, + HRESULT hr = S_OK; TRACE("(%p,%s,%s)\n", hwnd, debugstr_w(pszSubAppName), debugstr_w(pszSubIdList)); - hr = UXTHEME_SetWindowProperty(hwnd, atSubAppName, pszSubAppName); + if(!IsWindow(hwnd)) + hr = E_HANDLE; + if(SUCCEEDED(hr)) + hr = UXTHEME_SetWindowProperty(hwnd, atSubAppName, pszSubAppName); if(SUCCEEDED(hr)) hr = UXTHEME_SetWindowProperty(hwnd, atSubIdList, pszSubIdList); if(SUCCEEDED(hr)) This code is screaming for a goto. -- James Hawkins
2008/10/28 James Hawkins <truiken(a)gmail.com>:
+ HRESULT hr = S_OK; TRACE("(%p,%s,%s)\n", hwnd, debugstr_w(pszSubAppName), debugstr_w(pszSubIdList)); - hr = UXTHEME_SetWindowProperty(hwnd, atSubAppName, pszSubAppName); + if(!IsWindow(hwnd)) + hr = E_HANDLE; + if(SUCCEEDED(hr)) + hr = UXTHEME_SetWindowProperty(hwnd, atSubAppName, pszSubAppName); if(SUCCEEDED(hr)) hr = UXTHEME_SetWindowProperty(hwnd, atSubIdList, pszSubIdList); if(SUCCEEDED(hr))
This code is screaming for a goto.
I disagree. -- Rob Shearman
On Tue, Oct 28, 2008 at 10:17 AM, Rob Shearman <robertshearman(a)gmail.com> wrote:
2008/10/28 James Hawkins <truiken(a)gmail.com>:
+ HRESULT hr = S_OK; TRACE("(%p,%s,%s)\n", hwnd, debugstr_w(pszSubAppName), debugstr_w(pszSubIdList)); - hr = UXTHEME_SetWindowProperty(hwnd, atSubAppName, pszSubAppName); + if(!IsWindow(hwnd)) + hr = E_HANDLE; + if(SUCCEEDED(hr)) + hr = UXTHEME_SetWindowProperty(hwnd, atSubAppName, pszSubAppName); if(SUCCEEDED(hr)) hr = UXTHEME_SetWindowProperty(hwnd, atSubIdList, pszSubIdList); if(SUCCEEDED(hr))
This code is screaming for a goto.
I disagree.
Yes, it was late and my mind stopped after the next thought. There's nothing to clean up, so simply returning the failed result is optimal. -- James Hawkins
2008/10/28 James Hawkins <truiken(a)gmail.com>:
On Tue, Oct 28, 2008 at 10:17 AM, Rob Shearman <robertshearman(a)gmail.com> wrote:
2008/10/28 James Hawkins <truiken(a)gmail.com>:
This code is screaming for a goto.
I disagree.
Yes, it was late and my mind stopped after the next thought. There's nothing to clean up, so simply returning the failed result is optimal.
Ok, so I will use this version of the code but will hunt down where the hwnd is being used, as per Alexandre's comments. I'll send a revised patch later on today or tomorrow. Thanks, - Reece
participants (3)
-
James Hawkins -
Reece Dunn -
Rob Shearman