Re: [PATCH v8 3/3] shell32: Implement FolderItems_get_Count.

[Sebastian Lackner]

On 19.08.2016 11:43, Alex Henrie wrote:
> Cc: Sebastian Lackner <sebastian(a)fds-team.de>
>
> Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com>
> ---
>  dlls/shell32/shelldispatch.c       | 10 ++++++++--
>  dlls/shell32/tests/shelldispatch.c |  6 ------
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/dlls/shell32/shelldispatch.c b/dlls/shell32/shelldispatch.c
> index a456ad4..86b77d6 100644
> --- a/dlls/shell32/shelldispatch.c
> +++ b/dlls/shell32/shelldispatch.c
> @@ -1061,9 +1061,15 @@ static HRESULT WINAPI FolderItemsImpl_Invoke(FolderItems3 *iface,
>
> static HRESULT WINAPI FolderItemsImpl_get_Count(FolderItems3 *iface, LONG *count)
>  {
> -    FIXME("(%p,%p)\n", iface, count);
> +    FolderItemsImpl *This = impl_from_FolderItems(iface);
>
> -    return E_NOTIMPL;
> +    TRACE("(%p,%p)\n", iface, count);
> +
> +    if (!count)
> +        return E_INVALIDARG;

The tests contain a comment which suggests that Windows does not do such checks.
If this is true I would suggest to simplify it also here in your implementation.
> +
> +    *count = This->item_count;
> +    return S_OK;
>  }
>
> static HRESULT WINAPI FolderItemsImpl_get_Application(FolderItems3 *iface, IDispatch **ppid)
> diff --git a/dlls/shell32/tests/shelldispatch.c b/dlls/shell32/tests/shelldispatch.c
> index dc4fda4..42cfbf7 100644
> --- a/dlls/shell32/tests/shelldispatch.c
> +++ b/dlls/shell32/tests/shelldispatch.c
> @@ -378,9 +378,7 @@ static void test_items(void)
>          r = FolderItems_get_Count(items, NULL);
>
> r = FolderItems_get_Count(items, &lcount);
> -todo_wine
>      ok(r == S_OK, "FolderItems::get_Count failed: %08x\n", r);
> -todo_wine
>      ok(!lcount, "expected 0 files, got %d\n", lcount);
>
> V_VT(&var) = VT_I4;
> @@ -435,9 +433,7 @@ todo_wine
>
> lcount = -1;
>      r = FolderItems_get_Count(items, &lcount);
> -todo_wine
>      ok(r == S_OK, "FolderItems::get_Count failed: %08x\n", r);
> -todo_wine
>      ok(!lcount, "expected 0 files, got %d\n", lcount);
>      FolderItems_Release(items);
>
> @@ -455,9 +451,7 @@ todo_wine
>
> lcount = -1;
>      r = FolderItems_get_Count(items, &lcount);
> -todo_wine
>      ok(r == S_OK, "FolderItems::get_Count failed: %08x\n", r);
> -todo_wine
>      ok(lcount == sizeof(file_defs)/sizeof(file_defs[0]),
>         "expected %d files, got %d\n", sizeof(file_defs)/sizeof(file_defs[0]), lcount);

[PATCH v8 3/3] shell32: Implement FolderItems_get_Count.

[Alex Henrie]

2016-08-22 4:16 GMT-06:00 Sebastian Lackner <sebastian(a)fds-team.de>:
> On 19.08.2016 11:43, Alex Henrie wrote:
> > Cc: Sebastian Lackner <sebastian(a)fds-team.de>
> >
> > Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com>
> > ---
> >  dlls/shell32/shelldispatch.c       | 10 ++++++++--
> >  dlls/shell32/tests/shelldispatch.c |  6 ------
> >  2 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/dlls/shell32/shelldispatch.c b/dlls/shell32/shelldispatch.c
> > index a456ad4..86b77d6 100644
> > --- a/dlls/shell32/shelldispatch.c
> > +++ b/dlls/shell32/shelldispatch.c
> > @@ -1061,9 +1061,15 @@ static HRESULT WINAPI FolderItemsImpl_Invoke(FolderItems3 *iface,
> >
> > static HRESULT WINAPI FolderItemsImpl_get_Count(FolderItems3 *iface, LONG *count)
> >  {
> > -    FIXME("(%p,%p)\n", iface, count);
> > +    FolderItemsImpl *This = impl_from_FolderItems(iface);
> >
> > -    return E_NOTIMPL;
> > +    TRACE("(%p,%p)\n", iface, count);
> > +
> > +    if (!count)
> > +        return E_INVALIDARG;
>
> The tests contain a comment which suggests that Windows does not do such checks.
> If this is true I would suggest to simplify it also here in your implementation.

Good catch.

-Alex