New Coverity run Oct 1st. 58 fixed, 41 new warnings in cabinet, jscript, mshtml, wined3d, and code generated by widl

[Dan Kegel]

Thanks to the half-dozen Wine developers attacking the Coverity
warnings, we are slowly making progress.

Here are the warnings that were new as of October 1st
and which have not yet been classified.
They fall into five areas (four DLLs and one kind of generated code):

CID     Checker                   Func      File

765     FORWARD_NULL    FDICopy dlls/cabinet/fdi.c
766     FORWARD_NULL    FDICopy dlls/cabinet/fdi.c

770     NULL_RETURNS    alloc_array     dlls/jscript/array.c
771     NULL_RETURNS    Array_sort      dlls/jscript/array.c
773     NULL_RETURNS    JScriptFactory_CreateInstance   dlls/jscript/jscript.c
779     REVERSE_INULL   JScript_Close   dlls/jscript/jscript.c
780     REVERSE_INULL   JScript_Close   dlls/jscript/jscript.c
772     NULL_RETURNS    alloc_number    dlls/jscript/number.c
768     FORWARD_NULL    EmitREBytecode  dlls/jscript/regexp.c

767     FORWARD_NULL    _test_elem_getelembytag dlls/mshtml/tests/dom.c

509     FORWARD_NULL    drawStridedSlow dlls/wined3d/drawprim.c

781     REVERSE_INULL   __finally_IEnumFORMATETC_RemoteNext_Stub
 dlls/ole32/ole32_objidl_p.c
782     REVERSE_INULL   __finally_IEnumMoniker_RemoteNext_Stub
dlls/ole32/ole32_objidl_p.c
783     REVERSE_INULL   __finally_IEnumSTATDATA_RemoteNext_Stub
dlls/ole32/ole32_objidl_p.c
784     REVERSE_INULL   __finally_IEnumUnknown_RemoteNext_Stub
dlls/ole32/ole32_objidl_p.c
785     REVERSE_INULL   __finally_IDispatch_RemoteInvoke_Stub
dlls/oleaut32/oleaut32_oaidl_p.c
786     REVERSE_INULL   __finally_IEnumVARIANT_RemoteNext_Stub
dlls/oleaut32/oleaut32_oaidl_p.c
787     REVERSE_INULL   __finally_ITypeInfo_RemoteGetNames_Stub
dlls/oleaut32/oleaut32_oaidl_p.c
788     REVERSE_INULL   __finally_ITypeLib_RemoteFindName_Stub
dlls/oleaut32/oleaut32_oaidl_p.c
789     REVERSE_INULL
__finally_IEnumConnectionPoints_RemoteNext_Stub
dlls/oleaut32/oleaut32_ocidl_p.c
790     REVERSE_INULL   __finally_IEnumConnections_RemoteNext_Stub
 dlls/oleaut32/oleaut32_ocidl_p.c
791     REVERSE_INULL   __finally_IEnumOleUndoUnits_RemoteNext_Stub
 dlls/oleaut32/oleaut32_ocidl_p.c
792     REVERSE_INULL   __finally_IPropertyPage_SetObjects_Stub
dlls/oleaut32/oleaut32_ocidl_p.c
774     REVERSE_INULL   __finally_IEnumBackgroundCopyFiles_Next_Stub
 dlls/qmgrprxy/qmgrprxy_p.c
775     REVERSE_INULL   __finally_IEnumBackgroundCopyJobs_Next_Stub
 dlls/qmgrprxy/qmgrprxy_p.c
793     REVERSE_INULL   __finally_IServer_sum_padded2
dlls/rpcrt4/tests/server_s.c
794     REVERSE_INULL   __finally_IServer_sum_padded_conf
dlls/rpcrt4/tests/server_s.c
776     REVERSE_INULL   __finally_epm_ept_delete        programs/rpcss/epm_s.c
777     REVERSE_INULL   __finally_epm_ept_insert        programs/rpcss/epm_s.c
778     REVERSE_INULL   __finally_epm_ept_lookup        programs/rpcss/epm_s.c

For more info, see http://wiki.winehq.org/StaticAnalysis

[Rob Shearman]

2008/10/6 Dan Kegel <dank(a)kegel.com>:
> Thanks to the half-dozen Wine developers attacking the Coverity
> warnings, we are slowly making progress.
>
> Here are the warnings that were new as of October 1st
> and which have not yet been classified.
> They fall into five areas (four DLLs and one kind of generated code):
>
> CID     Checker                   Func      File
> ...
> 781     REVERSE_INULL   __finally_IEnumFORMATETC_RemoteNext_Stub
>  dlls/ole32/ole32_objidl_p.c
> 782     REVERSE_INULL   __finally_IEnumMoniker_RemoteNext_Stub
> dlls/ole32/ole32_objidl_p.c
> 783     REVERSE_INULL   __finally_IEnumSTATDATA_RemoteNext_Stub
> dlls/ole32/ole32_objidl_p.c
> 784     REVERSE_INULL   __finally_IEnumUnknown_RemoteNext_Stub
> dlls/ole32/ole32_objidl_p.c
> 785     REVERSE_INULL   __finally_IDispatch_RemoteInvoke_Stub
> dlls/oleaut32/oleaut32_oaidl_p.c
> 786     REVERSE_INULL   __finally_IEnumVARIANT_RemoteNext_Stub
> dlls/oleaut32/oleaut32_oaidl_p.c
> 787     REVERSE_INULL   __finally_ITypeInfo_RemoteGetNames_Stub
> dlls/oleaut32/oleaut32_oaidl_p.c
> 788     REVERSE_INULL   __finally_ITypeLib_RemoteFindName_Stub
> dlls/oleaut32/oleaut32_oaidl_p.c
> 789     REVERSE_INULL
> __finally_IEnumConnectionPoints_RemoteNext_Stub
> dlls/oleaut32/oleaut32_ocidl_p.c
> 790     REVERSE_INULL   __finally_IEnumConnections_RemoteNext_Stub
>  dlls/oleaut32/oleaut32_ocidl_p.c
> 791     REVERSE_INULL   __finally_IEnumOleUndoUnits_RemoteNext_Stub
>  dlls/oleaut32/oleaut32_ocidl_p.c
> 792     REVERSE_INULL   __finally_IPropertyPage_SetObjects_Stub
> dlls/oleaut32/oleaut32_ocidl_p.c
> 774     REVERSE_INULL   __finally_IEnumBackgroundCopyFiles_Next_Stub
>  dlls/qmgrprxy/qmgrprxy_p.c
> 775     REVERSE_INULL   __finally_IEnumBackgroundCopyJobs_Next_Stub
>  dlls/qmgrprxy/qmgrprxy_p.c
> 793     REVERSE_INULL   __finally_IServer_sum_padded2
> dlls/rpcrt4/tests/server_s.c
> 794     REVERSE_INULL   __finally_IServer_sum_padded_conf
> dlls/rpcrt4/tests/server_s.c
> 776     REVERSE_INULL   __finally_epm_ept_delete        programs/rpcss/epm_s.c
> 777     REVERSE_INULL   __finally_epm_ept_insert        programs/rpcss/epm_s.c
> 778     REVERSE_INULL   __finally_epm_ept_lookup        programs/rpcss/epm_s.c

These are caused by a bug in widl that generates the incorrect code.
Compare the code that widl generates:
static void __finally_IEnumMoniker_RemoteNext_Stub( struct
__frame_IEnumMoniker_RemoteNext_Stub *__frame )
{

    __frame->_StubMsg.MaxCount = (unsigned long)__frame->celt;

    __frame->_StubMsg.Offset = (unsigned long)0;
    __frame->_StubMsg.ActualCount = (unsigned long)*__frame->pceltFetched;

    NdrComplexArrayFree(
        &__frame->_StubMsg,
        (unsigned char *)__frame->rgelt,
        (PFORMAT_STRING)&__MIDL_TypeFormatString.Format[448]);
    if (__frame->rgelt)
        __frame->_StubMsg.pfnFree(__frame->rgelt);

    NdrPointerFree(
        &__frame->_StubMsg,
        (unsigned char *)__frame->pceltFetched,
        (PFORMAT_STRING)&__MIDL_TypeFormatString.Format[466]);

}

And the code that MIDL generates:
        {

        ( _StubMsg.MaxCount = ( unsigned long  )celt ) ;

        ( _StubMsg.Offset = ( unsigned long  )0 ) ;

        ( _StubMsg.ActualCount = ( unsigned long  )(pceltFetched ?
*pceltFetched : 0) ) ;



        NdrPointerFree( &_StubMsg,

                        (unsigned char *)rgelt,

                        &objidl__MIDL_TypeFormatString.Format[812] );



        }


So in this case and most of the cases like it we shouldn't be calling
NdrComplexFree directly. There are other cases where we could
potentially generate code that could crash, but the generated code is
similar to that generated by MIDL so it highlights a potential bug in
MIDL too.

However, it should be noted that these issues won't ever cause a
program to crash since we handle both types of exception in
CStdStubBuffer_Invoke. It's only a correctness issue as to what return
code we pass back to the client.

-- 
Rob Shearman

[Aric Stewart]

Do you have coverity access?  if so we can make all these ignore.

If not then tell me and I will go in and mark them ignore.

-aric

Rob Shearman wrote:
> 2008/10/6 Dan Kegel <dank(a)kegel.com>:
> > Thanks to the half-dozen Wine developers attacking the Coverity
> > warnings, we are slowly making progress.
> >
> > Here are the warnings that were new as of October 1st
> > and which have not yet been classified.
> > They fall into five areas (four DLLs and one kind of generated code):
> >
> > CID     Checker                   Func      File
> > ...
> > 781     REVERSE_INULL   __finally_IEnumFORMATETC_RemoteNext_Stub
> >  dlls/ole32/ole32_objidl_p.c
> > 782     REVERSE_INULL   __finally_IEnumMoniker_RemoteNext_Stub
> > dlls/ole32/ole32_objidl_p.c
> > 783     REVERSE_INULL   __finally_IEnumSTATDATA_RemoteNext_Stub
> > dlls/ole32/ole32_objidl_p.c
> > 784     REVERSE_INULL   __finally_IEnumUnknown_RemoteNext_Stub
> > dlls/ole32/ole32_objidl_p.c
> > 785     REVERSE_INULL   __finally_IDispatch_RemoteInvoke_Stub
> > dlls/oleaut32/oleaut32_oaidl_p.c
> > 786     REVERSE_INULL   __finally_IEnumVARIANT_RemoteNext_Stub
> > dlls/oleaut32/oleaut32_oaidl_p.c
> > 787     REVERSE_INULL   __finally_ITypeInfo_RemoteGetNames_Stub
> > dlls/oleaut32/oleaut32_oaidl_p.c
> > 788     REVERSE_INULL   __finally_ITypeLib_RemoteFindName_Stub
> > dlls/oleaut32/oleaut32_oaidl_p.c
> > 789     REVERSE_INULL
> > __finally_IEnumConnectionPoints_RemoteNext_Stub
> > dlls/oleaut32/oleaut32_ocidl_p.c
> > 790     REVERSE_INULL   __finally_IEnumConnections_RemoteNext_Stub
> >  dlls/oleaut32/oleaut32_ocidl_p.c
> > 791     REVERSE_INULL   __finally_IEnumOleUndoUnits_RemoteNext_Stub
> >  dlls/oleaut32/oleaut32_ocidl_p.c
> > 792     REVERSE_INULL   __finally_IPropertyPage_SetObjects_Stub
> > dlls/oleaut32/oleaut32_ocidl_p.c
> > 774     REVERSE_INULL   __finally_IEnumBackgroundCopyFiles_Next_Stub
> >  dlls/qmgrprxy/qmgrprxy_p.c
> > 775     REVERSE_INULL   __finally_IEnumBackgroundCopyJobs_Next_Stub
> >  dlls/qmgrprxy/qmgrprxy_p.c
> > 793     REVERSE_INULL   __finally_IServer_sum_padded2
> > dlls/rpcrt4/tests/server_s.c
> > 794     REVERSE_INULL   __finally_IServer_sum_padded_conf
> > dlls/rpcrt4/tests/server_s.c
> > 776     REVERSE_INULL   __finally_epm_ept_delete        programs/rpcss/epm_s.c
> > 777     REVERSE_INULL   __finally_epm_ept_insert        programs/rpcss/epm_s.c
> > 778     REVERSE_INULL   __finally_epm_ept_lookup        programs/rpcss/epm_s.c
>
> These are caused by a bug in widl that generates the incorrect code.
> Compare the code that widl generates:
> static void __finally_IEnumMoniker_RemoteNext_Stub( struct
> __frame_IEnumMoniker_RemoteNext_Stub *__frame )
> {
>
> __frame->_StubMsg.MaxCount = (unsigned long)__frame->celt;
>
> __frame->_StubMsg.Offset = (unsigned long)0;
>     __frame->_StubMsg.ActualCount = (unsigned long)*__frame->pceltFetched;
>
> NdrComplexArrayFree(
>         &__frame->_StubMsg,
>         (unsigned char *)__frame->rgelt,
>         (PFORMAT_STRING)&__MIDL_TypeFormatString.Format[448]);
>     if (__frame->rgelt)
>         __frame->_StubMsg.pfnFree(__frame->rgelt);
>
> NdrPointerFree(
>         &__frame->_StubMsg,
>         (unsigned char *)__frame->pceltFetched,
>         (PFORMAT_STRING)&__MIDL_TypeFormatString.Format[466]);
>
> }
>
> And the code that MIDL generates:
>         {
>
> ( _StubMsg.MaxCount = ( unsigned long  )celt ) ;
>
> ( _StubMsg.Offset = ( unsigned long  )0 ) ;
>
> ( _StubMsg.ActualCount = ( unsigned long  )(pceltFetched ?
> *pceltFetched : 0) ) ;
>
> NdrPointerFree( &_StubMsg,
>
> (unsigned char *)rgelt,
>
> &objidl__MIDL_TypeFormatString.Format[812] );
>
> }
>
> So in this case and most of the cases like it we shouldn't be calling
> NdrComplexFree directly. There are other cases where we could
> potentially generate code that could crash, but the generated code is
> similar to that generated by MIDL so it highlights a potential bug in
> MIDL too.
>
> However, it should be noted that these issues won't ever cause a
> program to crash since we handle both types of exception in
> CStdStubBuffer_Invoke. It's only a correctness issue as to what return
> code we pass back to the client.