[PATCH] quartz: Fix IEnumFilters::Skip() implementation.

[Zebediah Figura (she/her)]

On 11/23/20 7:30 AM, Gijs Vermeulen wrote:
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=28332
> Signed-off-by: Gijs Vermeulen <gijsvrm at gmail.com>
> ---
>  dlls/quartz/filtergraph.c       |  7 +++++--
>  dlls/quartz/tests/filtergraph.c | 27 +++++++++++++++++++++++++++
>  2 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/dlls/quartz/filtergraph.c b/dlls/quartz/filtergraph.c
> index b3aebabbdad..c9e26231032 100644
> --- a/dlls/quartz/filtergraph.c
> +++ b/dlls/quartz/filtergraph.c
> @@ -322,13 +322,16 @@ static HRESULT WINAPI EnumFilters_Skip(IEnumFilters *iface, ULONG count)
>  
>      TRACE("enum_filters %p, count %u.\n", enum_filters, count);
>  
> +    if (enum_filters->version != enum_filters->graph->version)
> +        return VFW_E_ENUM_OUT_OF_SYNC;
> +
>      if (!enum_filters->cursor)
> -        return S_FALSE;
> +        return E_INVALIDARG;
>  
>      while (count--)
>      {
>          if (!(enum_filters->cursor = list_next(&enum_filters->graph->filters, enum_filters->cursor)))
> -            return S_FALSE;
> +            return count ? S_FALSE : S_OK;
>      }
>  
>      return S_OK;

There's basically three separate changes here, and they could be split
up individually.

(In general, using "fix X" as a commit subject is not great; here it's a
sign that multiple different changes are being grouped together).

> diff --git a/dlls/quartz/tests/filtergraph.c b/dlls/quartz/tests/filtergraph.c
> index e7ee4e5c446..f35b54a02e8 100644
> --- a/dlls/quartz/tests/filtergraph.c
> +++ b/dlls/quartz/tests/filtergraph.c
> @@ -677,12 +677,39 @@ static void test_enum_filters(void)
>      hr = IEnumFilters_Next(enum1, 1, filters, NULL);
>      ok(hr == S_FALSE, "Got hr %#x.\n", hr);
>  
> +    hr = IEnumFilters_Skip(enum1, 0);
> +    ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
> +
> +    hr = IEnumFilters_Skip(enum1, 1);
> +    ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
> +
>      IFilterGraph2_AddFilter(graph, filter1, NULL);
>      IFilterGraph2_AddFilter(graph, filter2, NULL);
>  
>      hr = IEnumFilters_Next(enum1, 1, filters, NULL);
>      ok(hr == VFW_E_ENUM_OUT_OF_SYNC, "Got hr %#x.\n", hr);
>  
> +    hr = IEnumFilters_Skip(enum1, 1);
> +    ok(hr == VFW_E_ENUM_OUT_OF_SYNC, "Got hr %#x.\n", hr);
> +
> +    hr = IEnumFilters_Reset(enum1);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +
> +    hr = IEnumFilters_Skip(enum1, 0);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +
> +    hr = IEnumFilters_Skip(enum1, 1);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +
> +    hr = IEnumFilters_Skip(enum1, 2);
> +    ok(hr == S_FALSE, "Got hr %#x.\n", hr);

If you call Skip() again after this, does it return E_INVALIDARG? That
seems worth adding tests for.

> +
> +    hr = IEnumFilters_Reset(enum1);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +
> +    hr = IEnumFilters_Skip(enum1, 2);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +

Same here.

>      hr = IEnumFilters_Reset(enum1);
>      ok(hr == S_OK, "Got hr %#x.\n", hr);
>  
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 484 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20201123/820dce4f/attachment.sig>