Thread: [Mlt-devel] [PATCH 0/8] A few Coverity fixes
Brought to you by:
ddennedy,
lilo_booter
From: Mikko R. <mik...@ik...> - 2012-07-24 18:11:24
|
Tested these with kdenlive and nothing seems to break. Mikko Rapeli (8): riff.cc: Initialize data in constructor riff.cc: Fail if lseek() fails mlt_cache.c: check for null pointer mlt_cache.c: watch out for null pointer mlt_consumer.c: watch out for null pointer mlt_consumer_new(): handle return value from mlt_consumer_init() mlt_consumer_start(): get mutex before accessing put_active mlt_consumer_start(): check return value from mlt_properties_get_int() src/framework/mlt_cache.c | 5 ++++- src/framework/mlt_consumer.c | 32 ++++++++++++++++++++++++-------- src/modules/kino/riff.cc | 10 +++++++++- 3 files changed, 37 insertions(+), 10 deletions(-) -- 1.7.10.4 |
From: Mikko R. <mik...@ik...> - 2012-07-24 18:11:23
|
Fixes Coverity CID 709444: Uninitialized scalar field (UNINIT_CTOR) Non-static class member ""length"" is not initialized in this constructor nor in any functions that it calls. Non-static class member ""name"" is not initialized in this constructor nor in any functions that it calls. Non-static class member ""offset"" is not initialized in this constructor nor in any functions that it calls. Non-static class member ""parent"" is not initialized in this constructor nor in any functions that it calls. Non-static class member ""type"" is not initialized in this constructor nor in any functions that it calls. Non-static class member ""written"" is not initialized in this constructor nor in any functions that it calls. --- src/modules/kino/riff.cc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/modules/kino/riff.cc b/src/modules/kino/riff.cc index 58ecee3..abe4363 100644 --- a/src/modules/kino/riff.cc +++ b/src/modules/kino/riff.cc @@ -65,7 +65,14 @@ FOURCC make_fourcc( const char *s ) RIFFDirEntry::RIFFDirEntry() -{} +{ + type = 0; + name = 0; + length = 0; + offset = 0; + parent = 0; + written = 0; +} RIFFDirEntry::RIFFDirEntry ( FOURCC t, FOURCC n, int l, int o, int p ) : type( t ), name( n ), length( l ), offset( o ), parent( p ), written( 0 ) -- 1.7.10.4 |
From: Mikko R. <mik...@ik...> - 2012-07-24 18:11:23
|
Fixes Coverity CID 709393: Dereference before null check (REVERSE_INULL) Directly dereferencing pointer "profile". 235 profile->sample_aspect_num = mlt_properties_get_int( properties, "sample_aspect_num" ); Dereferencing "profile" before a null check. 236 if ( profile ) --- src/framework/mlt_consumer.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/framework/mlt_consumer.c b/src/framework/mlt_consumer.c index 9ff244b..de53a58 100644 --- a/src/framework/mlt_consumer.c +++ b/src/framework/mlt_consumer.c @@ -232,9 +232,11 @@ static void mlt_consumer_property_changed( mlt_properties owner, mlt_consumer se { mlt_properties properties = MLT_CONSUMER_PROPERTIES( self ); mlt_profile profile = mlt_service_profile( MLT_CONSUMER_SERVICE( self ) ); - profile->sample_aspect_num = mlt_properties_get_int( properties, "sample_aspect_num" ); if ( profile ) + { + profile->sample_aspect_num = mlt_properties_get_int( properties, "sample_aspect_num" ); mlt_properties_set_double( properties, "aspect_ratio", mlt_profile_sar( profile ) ); + } } else if ( !strcmp( name, "sample_aspect_den" ) ) { -- 1.7.10.4 |
From: Mikko R. <mik...@ik...> - 2012-07-24 18:11:26
|
Fixes Coverity CID 709343: Division or modulo by zero (DIVIDE_BY_ZERO) Division by expression "mlt_properties_get_int(properties, "frame_rate_num")" which may be zero has undefined behavior On this path, function call "mlt_properties_get_int(properties, "frame_rate_num")" has return value of 0 442 int frame_duration = 1000000 / mlt_properties_get_int( properties, "frame_rate_num" ) * 443 mlt_properties_get_int( properties, "frame_rate_den" ); --- src/framework/mlt_consumer.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/framework/mlt_consumer.c b/src/framework/mlt_consumer.c index 5ce2532..5334e94 100644 --- a/src/framework/mlt_consumer.c +++ b/src/framework/mlt_consumer.c @@ -448,8 +448,15 @@ int mlt_consumer_start( mlt_consumer self ) } // Set the frame duration in microseconds for the frame-dropping heuristic - int frame_duration = 1000000 / mlt_properties_get_int( properties, "frame_rate_num" ) * - mlt_properties_get_int( properties, "frame_rate_den" ); + int frame_rate_num = mlt_properties_get_int( properties, "frame_rate_num" ); + int frame_rate_den = mlt_properties_get_int( properties, "frame_rate_den" ); + int frame_duration = 0; + + if ( frame_rate_num && frame_rate_den ) + { + frame_duration = 1000000 / frame_rate_num * frame_rate_den; + } + mlt_properties_set_int( properties, "frame_duration", frame_duration ); // Check and run an ante command -- 1.7.10.4 |
From: Mikko R. <mik...@ik...> - 2012-07-24 18:11:24
|
Fixes CID 709392: Dereference before null check (REVERSE_INULL). --- src/framework/mlt_cache.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/framework/mlt_cache.c b/src/framework/mlt_cache.c index 7938f79..5a15868 100644 --- a/src/framework/mlt_cache.c +++ b/src/framework/mlt_cache.c @@ -260,6 +260,7 @@ void mlt_cache_close( mlt_cache cache ) void mlt_cache_purge( mlt_cache cache, void *object ) { + if (!cache) return; pthread_mutex_lock( &cache->mutex ); if ( cache && object ) { -- 1.7.10.4 |
From: Mikko R. <mik...@ik...> - 2012-07-24 18:11:27
|
If init() fails return NULL. Fixes Coverity CID 709325. At conditional (1): "self != NULL" taking the true branch. 339 if ( self != NULL ) CID 709325: Unchecked return value (CHECKED_RETURN) Calling function "mlt_consumer_init" without checking return value (as is done elsewhere 10 out of 11 times). No check of the return value of "mlt_consumer_init(self, NULL, profile)". 340 mlt_consumer_init( self, NULL, profile ); --- src/framework/mlt_consumer.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/framework/mlt_consumer.c b/src/framework/mlt_consumer.c index de53a58..08f990f 100644 --- a/src/framework/mlt_consumer.c +++ b/src/framework/mlt_consumer.c @@ -338,11 +338,16 @@ mlt_consumer mlt_consumer_new( mlt_profile profile ) mlt_consumer self = malloc( sizeof( struct mlt_consumer_s ) ); // Initialise it - if ( self != NULL ) - mlt_consumer_init( self, NULL, profile ); - - // Return it - return self; + if ( self != NULL && mlt_consumer_init( self, NULL, profile ) == 0 ) + { + // Return it + return self; + } + else + { + free(self); + return NULL; + } } /** Get the parent service object. -- 1.7.10.4 |
From: Mikko R. <mik...@ik...> - 2012-07-24 18:11:26
|
Fixes Coverity CID 709355: Data race condition (MISSING_LOCK) Accessing variable "self->put_active" (mlt_consumer_s.put_active) requires the mlt_consumer_s.put_mutex lock. 411 self->put_active = 1; --- src/framework/mlt_consumer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/framework/mlt_consumer.c b/src/framework/mlt_consumer.c index 08f990f..5ce2532 100644 --- a/src/framework/mlt_consumer.c +++ b/src/framework/mlt_consumer.c @@ -414,8 +414,10 @@ int mlt_consumer_start( mlt_consumer self ) char *test_card = mlt_properties_get( properties, "test_card" ); // Just to make sure nothing is hanging around... + pthread_mutex_lock( &self->put_mutex ); self->put = NULL; self->put_active = 1; + pthread_mutex_unlock( &self->put_mutex ); // Deal with it now. if ( test_card != NULL ) -- 1.7.10.4 |
From: Mikko R. <mik...@ik...> - 2012-07-24 18:11:27
|
Fixes Coverity CID 709362: Argument cannot be negative (NEGATIVE_RETURNS) Function "lseek(this->fd, 0LL, 0)" returns a negative number. Assigning: signed variable "pos" = "lseek". ... "pos" is passed to a parameter that cannot be negative. 548 fail_if( lseek( fd, pos, SEEK_SET ) == ( off_t ) - 1 ); --- src/modules/kino/riff.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/modules/kino/riff.cc b/src/modules/kino/riff.cc index abe4363..fff59ed 100644 --- a/src/modules/kino/riff.cc +++ b/src/modules/kino/riff.cc @@ -542,6 +542,7 @@ void RIFFFile::ParseRIFF( void ) int container = AddDirectoryEntry( make_fourcc( "FILE" ), make_fourcc( "FILE" ), 0, RIFF_NO_PARENT ); pos = lseek( fd, 0, SEEK_SET ); + fail_if( pos == -1 ); /* calculate file size from RIFF header instead from physical file. */ -- 1.7.10.4 |
From: Mikko R. <mik...@ik...> - 2012-07-24 18:11:32
|
Fixes Coverity CID 709346: Dereference after null check (FORWARD_NULL) Comparing "result" to null implies that "result" might be null. 449 if ( result && result->data ) 450 result->refcount++; Dereferencing null variable "result". 451 mlt_log( NULL, MLT_LOG_DEBUG, "%s: get %d = %p, %p\n", __FUNCTION__, cache->count - 1, *hit, result->data ); 452 --- src/framework/mlt_cache.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/framework/mlt_cache.c b/src/framework/mlt_cache.c index 5a15868..903ec59 100644 --- a/src/framework/mlt_cache.c +++ b/src/framework/mlt_cache.c @@ -448,8 +448,10 @@ mlt_cache_item mlt_cache_get( mlt_cache cache, void *object ) sprintf( key, "%p", *hit ); result = mlt_properties_get_data( cache->active, key, NULL ); if ( result && result->data ) + { result->refcount++; - mlt_log( NULL, MLT_LOG_DEBUG, "%s: get %d = %p, %p\n", __FUNCTION__, cache->count - 1, *hit, result->data ); + mlt_log( NULL, MLT_LOG_DEBUG, "%s: get %d = %p, %p\n", __FUNCTION__, cache->count - 1, *hit, result->data ); + } // swap the current array cache->current = alt; -- 1.7.10.4 |