Re: [PATCH v3 6/6] shell32: Set output interfaces to NULL in FolderItems functions.

[Michael Stefaniuc]

On 07/06/2016 06:55 AM, Alex Henrie wrote:
> Cc: Christian Costa 
> Cc: Sebastian Lackner
> Signed-off-by: Alex Henrie
>
> dlls/shell32/shelldispatch.c       | 9 ++++++++-
>  dlls/shell32/tests/shelldispatch.c | 3 ---
>  2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/dlls/shell32/shelldispatch.c b/dlls/shell32/shelldispatch.c
> index 6d7973e..2705389 100644
> --- a/dlls/shell32/shelldispatch.c
> +++ b/dlls/shell32/shelldispatch.c
> @@ -1063,12 +1063,16 @@ static HRESULT WINAPI FolderItemsImpl_get_Application(FolderItems3 *iface, IDisp
>      if (!ppid)
>          return E_INVALIDARG;
> - *ppid = NULL;
> return E_NOTIMPL;
>
> }
>
> static HRESULT WINAPI FolderItemsImpl_get_Parent(FolderItems3 *iface, IDispatch **ppid)
>  {
> - FIXME("(%p,%p)\n", iface, ppid);
> - TRACE("(%p,%p)\n", iface, ppid);
> -
> - if (ppid)
> -
>    *ppid = NULL;

This two if's look odd: one for ppid and then for !ppid.
Why don't you just unconditionally do the NULL assignment after the
check below?
>  if (!ppid)
>      return E_INVALIDARG;

bye
    michael

[PATCH v3 6/6] shell32: Set output interfaces to NULL in FolderItems functions.

[Alex Henrie]

2016-07-06 1:13 GMT-06:00 Michael Stefaniuc :
> On 07/06/2016 06:55 AM, Alex Henrie wrote:
> > [...]
>
> This two if's look odd: one for ppid and then for !ppid.
> Why don't you just unconditionally do the NULL assignment after the
> check below?
> >  if (!ppid)
> >      return E_INVALIDARG;

Thanks for catching that. This is a mistake, according to the tests
get_Parent should never return E_INVALIDARG.

-Alex

[PATCH v3 6/6] shell32: Set output interfaces to NULL in FolderItems functions.

[Alex Henrie]

2016-07-06 11:39 GMT-06:00 Alex Henrie :
> 2016-07-06 1:13 GMT-06:00 Michael Stefaniuc :
> > On 07/06/2016 06:55 AM, Alex Henrie wrote:
> > > [...]
> >
> > This two if's look odd: one for ppid and then for !ppid.
> > Why don't you just unconditionally do the NULL assignment after the
> > check below?
> > >  if (!ppid)
> > >      return E_INVALIDARG;
>
> Thanks for catching that. This is a mistake, according to the tests
> get_Parent should never return E_INVALIDARG.

The only difference between v3 and v4 of the patchset is the removal
of these two lines from FolderItemsImpl_get_Parent.

-Alex