[ros-dev] [ros-diff]rev 46907.gschneider[SHELL32] - Simplify checks for success of ShellExecuteEx

[victor martinez]

Hi, i have been giving a look to this patch and i have some doubts(just learning :) )
The code is:
@@ -1298,8 +1298,8 @@ static HRESULT WINAPI ICPanel_IContextMenu2_InvokeCommand(
        sei.hwnd = lpcmi->hwnd;
        sei.nShow = SW_SHOWNORMAL;
        sei.lpVerb = L"open";
-       ShellExecuteExW(&sei);
-       if (sei.hInstApp <= (HINSTANCE)32)
+
+       if (ShellExecuteExW(&sei) == FALSE)
           return E_FAIL;


MSDN says that if ShellExecuteExW fails it sets hInstApp to a value lower than 32
and,also, returns FALSE.
(MSDN: http://msdn.microsoft.com/en-us/library/bb762154(VS.85).aspx ) 
So both lines of codes(buggy and patched) are,at first sight,doing the same.Just using a
different way.
The Commit says: 
- Simplify checks for success of ShellExecuteEx, field hInst may be an unreliable
indicator according to 
http://msdn.microsoft.com/en-us/library/bb759784%28v=VS.85%29.aspx

Why is it unreliable? :) I am not saying it is reliable, just that i dont find the reason
there...my skills are limited.. :(
I have just found: "Although hInstApp is declared as an HINSTANCE for compatibility
with 16-bit Windows applications, it is not a true HINSTANCE. It can be cast only to an
int and compared to either 32 or the following SE_ERR_XXX error codes ".

So my question is: Is the Bug in the cast to (HINSTANCE) instead to (INT)?I mean,do this
solve the bug too?:
-       if (sei.hInstApp <= (HINSTANCE)32)
+       if ((INT)sei.hInstApp <= 32)

As you see i am just learning, so thanks in advance.
And yes, i prefer the new way that patch is using.Using the returned the value seems much
more logic than using a "collateral damage". :)
Btw, thanks Gregor for your hunting-fixing week ;)

 		 	   		  
_________________________________________________________________
¡Citas! ¡Ligues! ¿Salimos? ¿Cómo es tu pareja ideal? Búscala en el sitio nº1… ¡Regístrate
ya!
http://contactos.es.msn.com/?mtcmk=015352