Re: [PATCH 2/2] devenum: Implement IMoniker::IsEqual().

[Andrew Eikum]

On Sun, Mar 26, 2017 at 04:31:55PM +0800, Jactry Zeng wrote:
> +    res = ICreateDevEnum_CreateClassEnumerator(create_devenum, &CLSID_LegacyAmFilterCategory, &enum_moniker0, 0);
> +    ok(SUCCEEDED(res), "Cannot create enum moniker (res = %x)\n", res);
> +    res = ICreateDevEnum_CreateClassEnumerator(create_devenum, &CLSID_LegacyAmFilterCategory, &enum_moniker1, 0);
> +    ok(SUCCEEDED(res), "Cannot create enum moniker (res = %x)\n", res);
> +    if (!res)
> +    {
> +        if (!IEnumMoniker_Next(enum_moniker0, 1, &moniker0, NULL) && !IEnumMoniker_Next(enum_moniker1, 1, &moniker1, NULL))
> +        {
> +            res = IMoniker_IsEqual(moniker0, moniker1);
> +            ok(!res, "IMoniker_IsEqual should failed (res = %x)\n", res);
> +
> +            res = IMoniker_IsEqual(moniker1, moniker0);
> +            ok(!res, "IMoniker_IsEqual should failed (res = %x)\n", res);
> +
> +            IMoniker_Release(moniker0);
> +            IMoniker_Release(moniker1);
> +        }

These patches look fine to me except the ok() message is wrong on these two.

Also, I think it's easier to read code that explicitly compares against S_OK or
uses SUCCEEDED() instead of using !.

Andrew

[PATCH 2/2] devenum: Implement IMoniker::IsEqual().

[Jactry Zeng]

Hi Andrew,

Thanks for review! I sent another try.

2017-03-27 21:49 GMT+08:00 Andrew Eikum <aeikum(a)codeweavers.com>:
> On Sun, Mar 26, 2017 at 04:31:55PM +0800, Jactry Zeng wrote:
> > +    res = ICreateDevEnum_CreateClassEnumerator(create_devenum,
> > &CLSID_LegacyAmFilterCategory, &enum_moniker0, 0);
> > +    ok(SUCCEEDED(res), "Cannot create enum moniker (res = %x)\n", res);
> > +    res = ICreateDevEnum_CreateClassEnumerator(create_devenum,
> > &CLSID_LegacyAmFilterCategory, &enum_moniker1, 0);
> > +    ok(SUCCEEDED(res), "Cannot create enum moniker (res = %x)\n", res);
> > +    if (!res)
> > +    {
> > +        if (!IEnumMoniker_Next(enum_moniker0, 1, &moniker0, NULL) &&
> > !IEnumMoniker_Next(enum_moniker1, 1, &moniker1, NULL))
> > +        {
> > +            res = IMoniker_IsEqual(moniker0, moniker1);
> > +            ok(!res, "IMoniker_IsEqual should failed (res = %x)\n",
> > res);
> > +
> > +            res = IMoniker_IsEqual(moniker1, moniker0);
> > +            ok(!res, "IMoniker_IsEqual should failed (res = %x)\n",
> > res);
> > +
> > +            IMoniker_Release(moniker0);
> > +            IMoniker_Release(moniker1);
> > +        }
>
> These patches look fine to me except the ok() message is wrong on these
> two.
>
> Also, I think it's easier to read code that explicitly compares against
> S_OK or
> uses SUCCEEDED() instead of using !.
>
> Andrew

-- 
Regards,
Jactry Zeng