re: coverity ... new run finally
Marcus wrote:
A new Coverity run (274) was done finally, so we can restart looking at issues ;)
Still a lot of NULL ptr migration issues which suck to fix, but well.
Yay! Here are the new errors introduced since the end of May, sorted by source file. There are only 23 not in test code: 725 NULL_RETURNS Context_CopyProperties crypt32/context.c 715 FORWARD_NULL CryptMsgGetAndVerifySigner crypt32/msg.c 709 DEADCODE DEVENUM_ReadPinTypes devenum/createdevenum.c 717 FORWARD_NULL DEVENUM_ReadPinTypes devenum/createdevenum.c 726 OVERRUN_STATIC test_EnumGroupsInGroup dplayx/tests/dplayx.c 718 FORWARD_NULL MSI_GetUserInfo msi/msi.c 747 REVERSE_INULL ILockBytes_RemoteReadAt_Stub ole32/ole32_objidl_p.c 748 REVERSE_INULL ILockBytes_RemoteReadAt_Stub ole32/ole32_objidl_p.c 749 REVERSE_INULL ISequentialStream_RemoteRead_Stub ole32/ole32_objidl_p.c 750 REVERSE_INULL ISequentialStream_RemoteRead_Stub ole32/ole32_objidl_p.c 751 REVERSE_INULL IEnumOLEVERB_RemoteNext_Stub ole32/ole32_oleidl_p.c 752 REVERSE_INULL IEnumOLEVERB_RemoteNext_Stub ole32/ole32_oleidl_p.c 373 REVERSE_INULL IDispatch_GetIDsOfNames_Stub oleaut32/oleaut32_oaidl_p.c 753 REVERSE_INULL ITypeLib_RemoteFindName_Stub oleaut32/oleaut32_oaidl_p.c 754 REVERSE_INULL IEnumConnectionPoints_RemoteNext_Stub oleaut32/oleaut32_ocidl_p.c 755 REVERSE_INULL IEnumConnections_RemoteNext_Stub oleaut32/oleaut32_ocidl_p.c 714 FORWARD_NULL MediaSeekingImpl_SetPositions quartz/control.c 713 FORWARD_NULL TestFilter_Create quartz/tests/filtergraph.c 719 FORWARD_NULL ME_FindPixelPos riched20/caret.c 708 DEADCODE array_compute_and_size_conformance rpcrt4/ndr_marshall.c 720 FORWARD_NULL CreateContext wined3d/context.c 721 FORWARD_NULL CreateContext wined3d/context.c 722 FORWARD_NULL test_OpenRequest winhttp/tests/winhttp.c 723 FORWARD_NULL test_SendRequest winhttp/tests/winhttp.c 724 FORWARD_NULL test_WinHttpAddHeaders winhttp/tests/winhttp.c 716 FORWARD_NULL InternetCrackUrlW wininet/internet.c 712 FORWARD_NULL test_mmioOpen winmm/tests/mmio.c 710 FORWARD_NULL ldap_parse_sort_controlW wldap32/parse.c 711 FORWARD_NULL ldap_parse_vlv_controlW wldap32/parse.c Sure would be nice if Coverity could identify the cl which introduced each problem :-) - Dan
2008/9/9 Dan Kegel <dank(a)kegel.com>:
Marcus wrote:
A new Coverity run (274) was done finally, so we can restart looking at issues ;)
Still a lot of NULL ptr migration issues which suck to fix, but well.
Sure would be nice if Coverity could identify the cl which introduced each problem :-)
Given the line number you can always get that information using blame. Admittedly, this requires more work on our part, but it should be possible to write a script that can do this automatically. - Reece
Hi Dan,
There are only 23 not in test code:
am I confused by this statement? The following sure seem to be in test code, unless I misunderstand your meaning:
726 OVERRUN_STATIC test_EnumGroupsInGroup dplayx/tests/dplayx.c 722 FORWARD_NULL test_OpenRequest winhttp/tests/winhttp.c 723 FORWARD_NULL test_SendRequest winhttp/tests/winhttp.c 724 FORWARD_NULL test_WinHttpAddHeaders winhttp/tests/winhttp.c 712 FORWARD_NULL test_mmioOpen winmm/tests/mmio.c
--Juan
On Tue, Sep 9, 2008 at 8:51 AM, Juan Lang <juan.lang(a)gmail.com> wrote:
There are only 23 not in test code:
am I confused by this statement? The following sure seem to be in test code, unless I misunderstand your meaning:
726 OVERRUN_STATIC test_EnumGroupsInGroup dplayx/tests/dplayx.c 722 FORWARD_NULL test_OpenRequest winhttp/tests/winhttp.c 723 FORWARD_NULL test_SendRequest winhttp/tests/winhttp.c 724 FORWARD_NULL test_WinHttpAddHeaders winhttp/tests/winhttp.c 712 FORWARD_NULL test_mmioOpen winmm/tests/mmio.c
Did you count the ones I listed not in test code? I probably should have separated them out...
Did you count the ones I listed not in test code? I probably should have separated them out...
No, I didn't. Now I understand the source of my confusion, thanks. My account appears to be closed, so I hope you don't mind if I tell you the status of the two new crypt32 reports: 725 NULL_RETURNS Context_CopyProperties crypt32/context.c This one is valid, sort of. The returned property lists might indeed be NULL, but only if a) the app passes a bogus pointer, which will crash on Windows, b) Wine's code passes a bogus pointer, which is a bug elsewhere, or c) memory gets corrupted. As these are all exceptional conditions, I'll introduce an assert. 715 FORWARD_NULL CryptMsgGetAndVerifySigner crypt32/msg.c False positive, though it would be hard for a static analyzer to know it. I believe it's complaining about pdwSignerIndex being dereferenced on line 2848, when it's checked against NULL on line 2827, implying it might be allowed to be NULL. However it's only dereferenced if CMSG_USE_SIGNER_INDEX_FLAG is set, which implies that pdwSignerIndex must not be NULL. --Juan
alexd4 asked:
is there more details available on what and where it detects in the function (free of charge)?
Yes. See http://scan.coverity.com/devfaq.html#account - Dan
Dan Kegel wrote:
Still a lot of NULL ptr migration issues which suck to fix, but well. 709 DEADCODE DEVENUM_ReadPinTypes devenum/createdevenum.c 717 FORWARD_NULL DEVENUM_ReadPinTypes devenum/createdevenum.c
NULL-dereference may be a false positive from the way memory for lpMediaType is allocated there. Not sure though, will look some more later; BTW, a stupid question - is there more details available on what and where it detects in the function (free of charge)? Couldn't really find it by quick look around their website and google, except short definition of FORWARD_NULL meaning etc.
713 FORWARD_NULL TestFilter_Create quartz/tests/filtergraph.c
I sent a patch for this.
On Tue, Sep 09, 2008 at 09:12:55PM +0300, Alexander Dorofeyev wrote:
Dan Kegel wrote:
Still a lot of NULL ptr migration issues which suck to fix, but well. 709 DEADCODE DEVENUM_ReadPinTypes devenum/createdevenum.c 717 FORWARD_NULL DEVENUM_ReadPinTypes devenum/createdevenum.c
NULL-dereference may be a false positive from the way memory for lpMediaType is allocated there. Not sure though, will look some more later; BTW, a stupid question - is there more details available on what and where it detects in the function (free of charge)? Couldn't really find it by quick look around their website and google, except short definition of FORWARD_NULL meaning etc.
Of course, if you click on a specifiy ID it will show the sourcecode with the decisions the analyzer did, including the path where it thought this can happen. Ciao, Marcus
Marcus Meissner wrote:
On Tue, Sep 09, 2008 at 09:12:55PM +0300, Alexander Dorofeyev wrote:
Dan Kegel wrote:
Still a lot of NULL ptr migration issues which suck to fix, but well. 709 DEADCODE DEVENUM_ReadPinTypes devenum/createdevenum.c 717 FORWARD_NULL DEVENUM_ReadPinTypes devenum/createdevenum.c NULL-dereference may be a false positive from the way memory for lpMediaType is allocated there. Not sure though, will look some more later; BTW, a stupid question - is there more details available on what and where it detects in the function (free of charge)? Couldn't really find it by quick look around their website and google, except short definition of FORWARD_NULL meaning etc.
Of course, if you click on a specifiy ID it will show the sourcecode with the decisions the analyzer did, including the path where it thought this can happen.
Ciao, Marcus
But I don't think Alexander has an account. This is the list of currently defined users for Wine: mmcc Mike McCormack rollo Troy Rollo reif Robert Reif rshearman Robert Shearman scottb Scott Bambrough mhearn Mike Hearn cmorgan Chris Morgan jacek Jacek Caban detlef Detlef Riekenberg dank Daniel Kegel timoshkov Dmitry Timoshkov lenders Louis Lenders lats Jeff Latimer vriens Paul Vriens atalbot Andrew Talbot ploujnikov Michael Ploujnikov kblin Kai Blin jhawk James Hawkins jan.wine Jan Zerebecki stefan Stefan Dösinger marcus Marcus Meissner bvincent Brian Vincent juan Juan Lang -- Cheers, Paul.
participants (6)
-
Alexander Dorofeyev -
Dan Kegel -
Juan Lang -
Marcus Meissner -
Paul Vriens -
Reece Dunn