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.