Fix Coverity Static Analysis Issues in XBMC #1340

Merged
merged 68 commits into from Sep 1, 2012

Projects

None yet

4 participants

@kylhill
Team Kodi member

I've managed to get XBMC added to the list of Open Source projects that is scanned for free by the Coverity Static Analysis tool. See http://scan.coverity.com/ for more details about this program.

For other developers that are interested in viewing the static analysis output, please contact me for login information.

This pull request aggregates all of my commits to fix these static analysis issues. Please note that some of the uninitialized class member "fixes" may not resolve actual bugs, but instead follow good practice of initializing class members in the constructor.

Kyle Hill added some commits Aug 29, 2012
Kyle Hill Fix possible buffer overrun in DVDDemuxFFmpeg.cpp.
We should check that iStreamId is strictly less than MAX_STREAMS to
avoid reading past the end of the array.
ad9bc56
Kyle Hill Fix use after free of file pointer. 570b7cd
Kyle Hill Fix memory leak on control sequence parse error. 776d7bc
Kyle Hill Init Suffix and Prefix buffers.
Init Suffic and Prefix buffers in AnimatedGif.cpp to avoid possible
uninitialized reads.
172c101
Kyle Hill Fix memory leak in GUIFontTTFGL::CacheCharacter
Please note that it is safe to call delete on a NULL pointer.
783db04
Kyle Hill Fix leak in JpegIO::CreateThumbnailFromSurface d752f70
Kyle Hill Fix memory leak in CGUITextureManager::CanLoad a3c02a9
Kyle Hill Fix memory leak in CPerformanceStats::AddSample. 133bf76
Kyle Hill Fix memory leak in ScraperParser.cpp f830e63
Kyle Hill Fix possible uninitialized reads in ScraperUrl.cpp 62f0a06
Kyle Hill Fix memory leaks in VideoInfoScanner.cpp 7992a84
@jmarshallnz jmarshallnz was assigned Aug 29, 2012
Kyle Hill added some commits Aug 29, 2012
Kyle Hill Fix reversed NULL pointer check.
This NULL pointer check does not check for a NULL return from malloc()
before dereferencing the pointer.
960d85a
Kyle Hill Remove unused member from GUIAudioManager.h 6d571e1
Kyle Hill Removed unreachable code in GUIFixedListContainer d8bc33a
Kyle Hill Fix possible use of uninitialized variables in GUI
This changeset ensures that all class member variables are initialized
to sane defaults in all class constructors.
086d262
Kyle Hill Fix use of invalidated iterator in VAAPI.cpp 4b0cc49
Kyle Hill Fix missing break in VDPAU.cpp b8277b1
@elupus elupus commented on an outdated diff Sep 1, 2012
xbmc/cores/dvdplayer/DVDPlayerAudio.cpp
m_freq = CurrentHostFrequency();
+ memset(&m_decode, 0, sizeof(m_decode));
@elupus
elupus Sep 1, 2012

This is a class with functions. I'd prefer a clear function.

@elupus
elupus Sep 1, 2012

Just call m_decode->Release()

@elupus
elupus Sep 1, 2012

This is still wrong. Seems you didn't fix this up.

@elupus elupus commented on the diff Sep 1, 2012
xbmc/cores/paplayer/ADPCMCodec.cpp
@@ -54,7 +54,6 @@ bool ADPCMCodec::Init(const CStdString &strFile, unsigned int filecache)
m_BitsPerSample = 16;//m_dll.GetSampleSize(m_adpcm);
m_DataFormat = AE_FMT_S16NE;
m_TotalTime = m_dll.GetLength(m_adpcm); // fixme?
- m_iDataPos = 0;
@elupus elupus commented on an outdated diff Sep 1, 2012
xbmc/guilib/Key.cpp
@@ -193,8 +193,6 @@ void CKey::SetFromService(bool fromService)
m_amount[1] = posY;
m_amount[2] = offsetX;
m_amount[3] = offsetY;
- for (unsigned int i = 4; i < max_amounts; i++)
- m_amount[i] = 0;
@elupus
elupus Sep 1, 2012

If we ever increase max_amounts this would leak. It may not bee reachable now, but doesn't matter.

@elupus
Team Kodi member

If you can fix above, i'll merge this in this window.

@elupus elupus was assigned Sep 1, 2012
@elupus
Team Kodi member

Easiest to fix it up is to add a fixup commit, then rebase interactivily, sqaush the fixup commit into the original commit. The force push to to the same branch on your github and this pull request will update.

@kylhill
Team Kodi member

Thanks for the code review @elupus. There's still quite a few items that Coverity pointed out that I'd like to address, but I welcome getting this batch of changes merged into mainline.

@kylhill
Team Kodi member

Do you want me to squash my fixup commits back to their originals now, or is this PR ok as-is?

@elupus
Team Kodi member

I'd prefer if you fixed up the commits i commented on and force push again. then we don't get broken code in between.

Kyle Hill added some commits Aug 29, 2012
Kyle Hill Fixed missing breaks in DVDFactoryCodec.cpp 9dc906f
Kyle Hill Fix use of invalid iterator in POUtils.cpp 82ec399
Kyle Hill Check return of stat() syscall in XBMCTex.cpp 1eb2888
Kyle Hill Fix use of uninitialized member in NfoFile.h 9e2b37d
Kyle Hill Fix use of uninitialized variables in DVDCodecs 4dbacda
Kyle Hill Fix possible NULL dereferences in DVDPlayer.cpp b2b67ee
Kyle Hill Fix use of uninitialized data in dvdplayer. 93d35bd
Kyle Hill Prevent possible integer overflow in Edl.cpp c6de7b0
Kyle Hill Fix possible integer overflow in CDDAcodec.cpp 2aab7dd
Kyle Hill Fix invalid virtual override in CachingCodec.h 8bbb5c9
Kyle Hill Fix possible NULL dereference in GUIKeyboardFactory.cpp 0394a55
Kyle Hill Check return of fseek in TextureBundleXPR.cpp 4b231b3
Kyle Hill Fix use of uninitialized member in ThreadLocal.h
It is possible for pthread_key_create to fail to initialize key on an
error.  We should set it to some default value to make debugging easier
if we run into this condition.
98f5c67
Kyle Hill Check that CFile::Open was successful
Before attempting to read from CFile, check that Open() was successful,
if not we'll skip over the item in our cache and refetch the resource.
1d65e4e
Kyle Hill Fix use of uninitialized variables in utils. 0f3bd45
Kyle Hill Check return values for error in VideoDatabase.cpp 6aad439
Kyle Hill Fix use of uninitialized variables in video/ 4e3cc69
Kyle Hill Removed if check with empty body from VDPAU.cpp
Coverity reported this as a stray semicolon. Modify line to match how
this check is performed elsewhere.
3c361d0
Kyle Hill Remove unreachable code in DVDInputStreamNavigator f79bd4e
Kyle Hill Fix uninitialzed variables in WAVcodec.cpp 9a75159
Kyle Hill Fix uninitialized m_dll in TimidityCodec.cpp 5825a42
Kyle Hill Remove unused member from PCMCodec.h 301fc90
Kyle Hill Fix uninitialized member in PAPlayer.cpp 82b44fc
Kyle Hill Fix uninitialized variable in OGGcodec.cpp 5500ac1
Kyle Hill Fix uninitialized data in NSFCodec.cpp 28b670f
Kyle Hill Fix uninitialized variables in MP3Codec 0245a18
Kyle Hill Fix uninitialized members in ICodec.h c6945dc
Kyle Hill Fix uninitialized member in DVDPlayerCodec.cpp b589296
Kyle Hill Remove unused member from ADPCMCodec a92ad71
Kyle Hill Fix uninitialized member in DVDSubtitlesLibass.cpp c20eca0
Kyle Hill Fix uninitialized member in DVDSubtitleParserVplayer fdb1d3f
Kyle Hill Fix uninitialized member in DVDSubtitleParserMicroDVD 0c6a8ee
Kyle Hill Fix uninitialized member in DVDSubtitleParserMPL2 b7caa26
Kyle Hill Fix use of uninitialized members in dvdplayer/ 43b0cf6
Kyle Hill Fixed missing NULL check in DVDPlayer.cpp 4b121de
Kyle Hill Remove unneeded NULL check from DVDDemuxFFmpeg.cpp 1c17143
Kyle Hill Fix buffer overrun in VDPAU.h
We can potentially have 13 VDP features enabled and we will overrun the
end of the m_features array if more than 10 are enabled at once.
535c4ac
Kyle Hill Fix possible improper NULL string termination
Make sure that the language string is always terminated by a NULL
character.  strncpy will copy at most 4 characters, but no NULL
character is implicitly appended so language will only be NULL
terminated if the string has fewer than 4 characers.
0568eda
Kyle Hill Fix uninitialized reads in AnimatedGif.cpp
The x member of the COLOR struct was never initialized in
AnimatedGif.cpp.
19f481c
Kyle Hill Fix Coverity issues in Weather.cpp
This changeset fixes possible uninitialized reads and NULL pointer
dereferences in Weather.cpp
8d305fe
Kyle Hill Fix uninitialized members in XBPython 742d6bd
Kyle Hill Remove unused variables from udf25.cpp cfc3006
Kyle Hill Fix use of uninitialized members of XBPyThread.cpp db45798
Kyle Hill Fix use of uninitialized members in GUIInfoManager.cpp effc642
Kyle Hill Fix uninitialized members in Application.cpp 1de4ec8
@kylhill
Team Kodi member

@elupus, I think I got it this time. Let me know if you have other feedback. Thanks.

Kyle Hill added some commits Sep 1, 2012
Kyle Hill Prevent possible NULL pointer dereferences. 7ff9e0c
Kyle Hill Fixed use of temporary pointer to string in freed vector
In Visualisation.cpp:213, a string pointer to an item in this temporary
vector is used.  Changing this vector to return by reference, as other
vectors in this class are, resolve this.
486c469
@elupus
Team Kodi member

Looks good. Will look over it once more, then merge if okey.

@elupus elupus and 1 other commented on an outdated diff Sep 1, 2012
xbmc/guilib/Shader.h
@@ -141,7 +141,6 @@
{
Free();
delete m_pFP;
- delete m_pVP;
@elupus
elupus Sep 1, 2012

This is wrong. Nothing else free's this in the base. Any overload need to set it to null if it frees it before.

@kylhill
kylhill Sep 1, 2012

Yeah, I don't know what I was thinking there.

@elupus
Team Kodi member

Sorry I missed above in last review. You were so quick to respond to changes. I can merge it in manually dropping the Shader.h change, but i can't press the nice big green merge button :).

@kylhill
Team Kodi member

I've remove the unused member now. Sorry about these new commits slipping in. I've been working away on this branch and don't want to lose these changes when I rebase.

@elupus elupus merged commit d0911de into xbmc:master Sep 1, 2012
@LongChair LongChair added a commit to plexinc/plex-home-theater-public that referenced this pull request Nov 21, 2014
@LongChair LongChair Dont use url options for PQ URIs, fixes #1340 a859f7d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment