Re: [PATCH] [shell32] Implement SHRunControlPanel using ShellExecute

[Nikolay Sivov]

On 26.02.2015 17:01, Robert Naumann wrote:
> ---
>   dlls/shell32/shellord.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/shell32/shellord.c b/dlls/shell32/shellord.c
> index 914dc5a..9ea3d62 100644
> --- a/dlls/shell32/shellord.c
> +++ b/dlls/shell32/shellord.c
> @@ -1129,8 +1129,10 @@ HRESULT WINAPI SHWinHelp (DWORD v, DWORD w, DWORD x, DWORD z)
>    */
>   BOOL WINAPI SHRunControlPanel (LPCWSTR commandLine, HWND parent)
>   {
> -	FIXME("(%s, %p): stub\n", debugstr_w(commandLine), parent);
> -	return FALSE;
> +    if((int)ShellExecute(parent,NULL,commandLine,NULL,NULL,SW_NORMAL)>32)
> +        return TRUE;
> +    else
> +        return FALSE;
>   }

Why 32? Also please don't remove tracing line.

[PATCH] [shell32] Implement SHRunControlPanel using ShellExecute

[Alex Henrie]

2015-02-26 7:36 GMT-07:00 Nikolay Sivov <bunglehead(a)gmail.com>:
> On 26.02.2015 17:01, Robert Naumann wrote:
> > -       FIXME("(%s, %p): stub\n", debugstr_w(commandLine), parent);
> > -       return FALSE;
> > +    if((int)ShellExecute(parent,NULL,commandLine,NULL,NULL,SW_NORMAL)>32)
> > +        return TRUE;
> > +    else
> > +        return FALSE;
>
> Why 32? Also please don't remove tracing line.

MSDN says that ShellExecute returns a value of 32 or less if it fails,
and a value greater than 32 if it succeeds:
https://msdn.microsoft.com/en-us/library/windows/desktop/bb762153%28v=vs.85%...

-Alex

[PATCH] [shell32] Implement SHRunControlPanel using ShellExecute

[Alex Henrie]

+    if((int)ShellExecute(parent,NULL,commandLine,NULL,NULL,SW_NORMAL)>32)
+        return TRUE;
+    else
+        return FALSE;

I think this would be more elegantly expressed as

return (int)ShellExecute(parent,NULL,commandLine,NULL,NULL,SW_NORMAL)>32;

But more importantly, are you sure that SHRunControlPanel will execute
anything? What if the filename doesn't end in .cpl? You'll probably
have to write some tests before this patch is accepted.

-Alex