Hi Victor,
you are perfectly right here, the situation is complicated. In my commit msg I provided a link, which specifies the following behaviour:
hInstApp HINSTANCEThat made me think: what happens to this field if the ShellExecuteEx call succeeds and the flag SEE_MASK_NOCLOSEPROCESS is not set? Since this check was thought to forward the actual result of the execution i switched to a simpler and (in my eyes) more reliable code. Plus the original code did not use the flag noted in the specification.
[out] If SEE_MASK_NOCLOSEPROCESS is set and the ShellExecuteEx call succeeds, it sets this member to a value greater than 32. If the function fails, it is set to an SE_ERR_XXX error value that indicates the cause of the failure.
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 ;)
Tus datos personales, más seguros con Internet Explorer 8. ¡Descárgatelo gratis!
_______________________________________________
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev