Re: [PATCH 1/2] oleaut32/tests: Add test for ITypeInfo2_fnGetContainingTypeLib

[Nikolay Sivov]

On 11/7/2012 11:36, Tatyana Fokina wrote:

Hi, Tatyana.

Some comments:
> dlls/oleaut32/tests/Makefile.in |  1 +
>   dlls/oleaut32/tests/typelib2.c  | 69 +++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 70 insertions(+)
>   create mode 100644 dlls/oleaut32/tests/typelib2.c
> You don't need a separate file for this, please use typelib.c
> +    hr = CreateTypeLib2(SYS_WIN32, jsdeb, (ICreateTypeLib2 **)&ctl2);
> +    if(hr != S_OK)
> +    {
> +        skip ("CreateTypeLib2 failed\n");
> +        return;
> +    }
> +
> +    hr = ICreateTypeLib2_CreateTypeInfo(ctl2, jsname, TKIND_DISPATCH, &cti);
> +    if(hr != S_OK)
> +    {
> +        skip ("ICreateTypeLib2_CreateTypeInfo failed\n");
> +        return;
> +    }
> That's not what skip() is for. Also you leak on second return and you 
> don't need a cast for ctl2.
> +    hr = ITypeInfo2_GetContainingTypeLib(ti2, (ITypeLib **)&tl, &Index);
> +    ok(!hr, " ITypeInfo2_GetContainingTypeLib failed on Index\n");
> +    if(!hr)ITypeLib_Release(tl);
> Please test return value for S_OK here, again you don't need a cast here.
> +    hr = ITypeInfo2_GetContainingTypeLib(ti2, (ITypeLib **)&tl, &Index);
> +    ok(!hr, " ITypeInfo2_GetContainingTypeLib failed on Index\n");
> +    if(!hr)ITypeLib_Release(tl);
> +
> +    hr = ITypeInfo2_GetContainingTypeLib(ti2, (ITypeLib **)&tl, pIndex);
> +    ok(!hr, " ITypeInfo2_GetContainingTypeLib failed on pIndex = NULL\n");
> +    if(!hr)ITypeLib_Release(tl);
> What are you trying to do with index pointer?

[PATCH 1/2] oleaut32/tests: Add test for ITypeInfo2_fnGetContainingTypeLib

[Tatyana Fokina]

Nikolay Sivov wrote 07.11.2012 13:45:
> On 11/7/2012 11:36, Tatyana Fokina wrote:
>
> Hi, Tatyana.
>
> Some comments:
> > dlls/oleaut32/tests/Makefile.in |  1 +
> >   dlls/oleaut32/tests/typelib2.c  | 69 
> > +++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 70 insertions(+)
> >   create mode 100644 dlls/oleaut32/tests/typelib2.c
> > You don't need a separate file for this, please use typelib.c
> > +    hr = CreateTypeLib2(SYS_WIN32, jsdeb, (ICreateTypeLib2 
> > **)&ctl2);
> > +    if(hr != S_OK)
> > +    {
> > +        skip ("CreateTypeLib2 failed\n");
> > +        return;
> > +    }
> > +
> > +    hr = ICreateTypeLib2_CreateTypeInfo(ctl2, jsname, 
> > TKIND_DISPATCH, &cti);
> > +    if(hr != S_OK)
> > +    {
> > +        skip ("ICreateTypeLib2_CreateTypeInfo failed\n");
> > +        return;
> > +    }
> > That's not what skip() is for. Also you leak on second return and you
> > don't need a cast for ctl2.
> > +    hr = ITypeInfo2_GetContainingTypeLib(ti2, (ITypeLib **)&tl, 
> > &Index);
> > +    ok(!hr, " ITypeInfo2_GetContainingTypeLib failed on Index\n");
> > +    if(!hr)ITypeLib_Release(tl);
> > Please test return value for S_OK here, again you don't need a cast 
> > here.
> > +    hr = ITypeInfo2_GetContainingTypeLib(ti2, (ITypeLib **)&tl, 
> > &Index);
> > +    ok(!hr, " ITypeInfo2_GetContainingTypeLib failed on Index\n");
> > +    if(!hr)ITypeLib_Release(tl);
> > +
> > +    hr = ITypeInfo2_GetContainingTypeLib(ti2, (ITypeLib **)&tl, 
> > pIndex);
> > +    ok(!hr, " ITypeInfo2_GetContainingTypeLib failed on pIndex = 
> > NULL\n");
> > +    if(!hr)ITypeLib_Release(tl);
> > What are you trying to do with index pointer?

Hi, Nikolay.

Thank you for your response. I fixed it.