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 HINSTANCE [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. That 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. The link you provided on the other hand makes it look like ShellExecuteEx always sets this field indendent of any flags passed to the function. To find out the truth one would have to write test cases. It might well be that the implementation we use is partially incorrect. Especially when looking at testman (it works again :-O ) @ shell32_winetest shlexec which still has some failures. Greg 2010/4/19 victor martinez <[email protected]> > 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<http://msdn.microsoft.com/en-us/library/bb762154%28VS.85%29.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!<http://www.microsoft.com/spain/windows/internet-explorer/default.aspx> > > _______________________________________________ > Ros-dev mailing list > [email protected] > http://www.reactos.org/mailman/listinfo/ros-dev >
_______________________________________________ Ros-dev mailing list [email protected] http://www.reactos.org/mailman/listinfo/ros-dev
