Re: [PATCH] shell32: Implement IShellDispatch2::ShellExecute

[Dmitry Timoshkov]

Andrew Eikum  wrote:
> - TRACE("(%s, %s, %s, %s, %s): stub\n", debugstr_w(file), debugstr_variant(&v_args),
> -
>        debugstr_variant(&v_dir), debugstr_variant(&v_op), debugstr_variant(&v_show));

Probably ": stub" part should be removed.
> - if (V_VT(&v_op) == VT_BSTR)
> -
>    op = V_BSTR(&v_op);
> -
> - if (V_VT(&v_args) == VT_BSTR)
> -
>    args = V_BSTR(&v_args);
> -
> - if (V_VT(&v_dir) == VT_BSTR)
> -
>    dir = V_BSTR(&v_dir);
> -
> - if (V_VT(&v_show) == VT_INT)
> -
>    show = V_INT(&v_show);
> -
> - ShellExecuteW(NULL, op, file, args, dir, show);
> -
> - return S_OK;

Shouldn't return value somehow reflect what ShellExecute returns?
Also as usually it would be nice to see some tests for this, especially
with not handled variant types.

[PATCH] shell32: Implement IShellDispatch2::ShellExecute

[Andrew Eikum]

On Fri, Oct 16, 2015 at 11:28:27PM +0800, Dmitry Timoshkov wrote:
> Andrew Eikum  wrote:
> > - TRACE("(%s, %s, %s, %s, %s): stub\n", debugstr_w(file), debugstr_variant(&v_args),
> > -
> >        debugstr_variant(&v_dir), debugstr_variant(&v_op), debugstr_variant(&v_show));
>
> Probably ": stub" part should be removed.

Yeah, it should.
> > - if (V_VT(&v_op) == VT_BSTR)
> > -
> >    op = V_BSTR(&v_op);
> > -
> > - if (V_VT(&v_args) == VT_BSTR)
> > -
> >    args = V_BSTR(&v_args);
> > -
> > - if (V_VT(&v_dir) == VT_BSTR)
> > -
> >    dir = V_BSTR(&v_dir);
> > -
> > - if (V_VT(&v_show) == VT_INT)
> > -
> >    show = V_INT(&v_show);
> > -
> > - ShellExecuteW(NULL, op, file, args, dir, show);
> > -
> > - return S_OK;
>
> Shouldn't return value somehow reflect what ShellExecute returns?
> Also as usually it would be nice to see some tests for this, especially
> with not handled variant types.

I wanted to avoid re-testing all of ShellExecute itself and verifying
the errors, but it's a good idea to test bad variants. I'll see what I
can do.

Thanks,
Andrew