From 391316fee29e40eff03baca2c6e601f1703e0681 Mon Sep 17 00:00:00 2001 From: Scott Talbot Date: Mon, 3 Feb 2014 18:52:00 +1100 Subject: [PATCH] Don't dereference NULL, ensure HashChain fully initialized Found by clang's static analyzer, they look validly uninitialized to me. Change-Id: I650250f516cdf6081b35cdfe92288c20a3036ac8 --- src/enc/backward_references.c | 25 +++++++++++++------------ src/enc/vp8l.c | 4 +++- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/enc/backward_references.c b/src/enc/backward_references.c index 77b4be74..a6ddeae4 100644 --- a/src/enc/backward_references.c +++ b/src/enc/backward_references.c @@ -113,11 +113,16 @@ static WEBP_INLINE uint64_t GetPixPairHash64(const uint32_t* const argb) { return key; } -static int HashChainInit(HashChain* const p, int size) { +static HashChain* HashChainNew(int size) { int i; + HashChain* const p = (HashChain*)malloc(sizeof(*p)); + if (p == NULL) { + return NULL; + } p->chain_ = (int*)WebPSafeMalloc((uint64_t)size, sizeof(*p->chain_)); if (p->chain_ == NULL) { - return 0; + free(p); + return NULL; } for (i = 0; i < size; ++i) { p->chain_[i] = -1; @@ -125,7 +130,7 @@ static int HashChainInit(HashChain* const p, int size) { for (i = 0; i < HASH_SIZE; ++i) { p->hash_to_first_index_[i] = -1; } - return 1; + return p; } static void HashChainDelete(HashChain* const p) { @@ -278,7 +283,7 @@ static int BackwardReferencesHashChain(int xsize, int ysize, int cc_init = 0; const int use_color_cache = (cache_bits > 0); const int pix_count = xsize * ysize; - HashChain* const hash_chain = (HashChain*)malloc(sizeof(*hash_chain)); + HashChain* const hash_chain = HashChainNew(pix_count); VP8LColorCache hashers; int window_size = WINDOW_SIZE; int iter_pos = 1; @@ -290,7 +295,6 @@ static int BackwardReferencesHashChain(int xsize, int ysize, if (!cc_init) goto Error; } - if (!HashChainInit(hash_chain, pix_count)) goto Error; refs->size = 0; GetParamsForHashChainFindCopy(quality, xsize, cache_bits, @@ -485,7 +489,7 @@ static int BackwardReferencesHashChainDistanceOnly( float* const cost = (float*)WebPSafeMalloc((uint64_t)pix_count, sizeof(*cost)); CostModel* cost_model = (CostModel*)malloc(sizeof(*cost_model)); - HashChain* hash_chain = (HashChain*)malloc(sizeof(*hash_chain)); + HashChain* hash_chain = HashChainNew(pix_count); VP8LColorCache hashers; const double mul0 = (recursive_cost_model != 0) ? 1.0 : 0.68; const double mul1 = (recursive_cost_model != 0) ? 1.0 : 0.82; @@ -496,8 +500,6 @@ static int BackwardReferencesHashChainDistanceOnly( if (cost == NULL || cost_model == NULL || hash_chain == NULL) goto Error; - if (!HashChainInit(hash_chain, pix_count)) goto Error; - if (use_color_cache) { cc_init = VP8LColorCacheInit(&hashers, cache_bits); if (!cc_init) goto Error; @@ -633,12 +635,11 @@ static int BackwardReferencesHashChainFollowChosenPath( int window_size = WINDOW_SIZE; int iter_pos = 1; int iter_limit = -1; - HashChain* hash_chain = (HashChain*)malloc(sizeof(*hash_chain)); + HashChain* hash_chain = HashChainNew(pix_count); VP8LColorCache hashers; - if (hash_chain == NULL || !HashChainInit(hash_chain, pix_count)) { - goto Error; - } + if (hash_chain == NULL) goto Error; + if (use_color_cache) { cc_init = VP8LColorCacheInit(&hashers, cache_bits); if (!cc_init) goto Error; diff --git a/src/enc/vp8l.c b/src/enc/vp8l.c index 15726318..27bd9bb6 100644 --- a/src/enc/vp8l.c +++ b/src/enc/vp8l.c @@ -959,7 +959,9 @@ static VP8LEncoder* VP8LEncoderNew(const WebPConfig* const config, } static void VP8LEncoderDelete(VP8LEncoder* enc) { - free(enc->argb_); + if (enc != NULL) { + free(enc->argb_); + } free(enc); }