From bfda85f2ee30095070c062b930994f645aec99b7 Mon Sep 17 00:00:00 2001 From: "bjornv@webrtc.org" Date: Mon, 16 Apr 2012 07:28:29 +0000 Subject: [PATCH] Safe handling of invalid inputs in delay_estimator. The delay_estimator crash on invalid create inputs when running new unit tests. This can't occur on a higher level unless corresponding enum and defines are incorrectly changed. The create and free functions are now more like malloc() and free() in design. The complete change to that will be done in a seperate CL. BUG= TEST= Review URL: https://webrtc-codereview.appspot.com/492003 git-svn-id: http://webrtc.googlecode.com/svn/trunk@2027 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../utility/delay_estimator.c | 73 +++++++++---------- .../utility/delay_estimator.h | 4 +- .../utility/delay_estimator_wrapper.c | 45 ++++++------ .../utility/delay_estimator_wrapper.h | 4 +- 4 files changed, 58 insertions(+), 68 deletions(-) diff --git a/src/modules/audio_processing/utility/delay_estimator.c b/src/modules/audio_processing/utility/delay_estimator.c index 24ee74d7f5..d74842116c 100644 --- a/src/modules/audio_processing/utility/delay_estimator.c +++ b/src/modules/audio_processing/utility/delay_estimator.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011 The WebRTC project authors. All Rights Reserved. + * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved. * * Use of this source code is governed by a BSD-style license * that can be found in the LICENSE file in the root of the source @@ -59,33 +59,28 @@ static void BitCountComparison(uint32_t binary_vector, } } -int WebRtc_FreeBinaryDelayEstimator(BinaryDelayEstimator* handle) { - assert(handle != NULL); +void WebRtc_FreeBinaryDelayEstimator(BinaryDelayEstimator* handle) { - if (handle->mean_bit_counts != NULL) { - free(handle->mean_bit_counts); - handle->mean_bit_counts = NULL; - } - if (handle->bit_counts != NULL) { - free(handle->bit_counts); - handle->bit_counts = NULL; - } - if (handle->binary_far_history != NULL) { - free(handle->binary_far_history); - handle->binary_far_history = NULL; - } - if (handle->binary_near_history != NULL) { - free(handle->binary_near_history); - handle->binary_near_history = NULL; - } - if (handle->far_bit_counts != NULL) { - free(handle->far_bit_counts); - handle->far_bit_counts = NULL; + if (handle == NULL) { + return; } + free(handle->mean_bit_counts); + handle->mean_bit_counts = NULL; + + free(handle->bit_counts); + handle->bit_counts = NULL; + + free(handle->binary_far_history); + handle->binary_far_history = NULL; + + free(handle->binary_near_history); + handle->binary_near_history = NULL; + + free(handle->far_bit_counts); + handle->far_bit_counts = NULL; + free(handle); - - return 0; } int WebRtc_CreateBinaryDelayEstimator(BinaryDelayEstimator** handle, @@ -93,10 +88,13 @@ int WebRtc_CreateBinaryDelayEstimator(BinaryDelayEstimator** handle, int lookahead) { BinaryDelayEstimator* self = NULL; int history_size = max_delay + lookahead; + int return_value = 0; if (handle == NULL) { return -1; } + *handle = NULL; + if (max_delay < 0) { return -1; } @@ -118,6 +116,7 @@ int WebRtc_CreateBinaryDelayEstimator(BinaryDelayEstimator** handle, self->bit_counts = NULL; self->binary_far_history = NULL; self->far_bit_counts = NULL; + self->binary_near_history = NULL; self->history_size = history_size; self->near_history_size = lookahead + 1; @@ -125,38 +124,32 @@ int WebRtc_CreateBinaryDelayEstimator(BinaryDelayEstimator** handle, // Allocate memory for spectrum buffers. self->mean_bit_counts = malloc(history_size * sizeof(int32_t)); if (self->mean_bit_counts == NULL) { - WebRtc_FreeBinaryDelayEstimator(self); - self = NULL; - return -1; + return_value = -1; } self->bit_counts = malloc(history_size * sizeof(int32_t)); if (self->bit_counts == NULL) { - WebRtc_FreeBinaryDelayEstimator(self); - self = NULL; - return -1; + return_value = -1; } // Allocate memory for history buffers. self->binary_far_history = malloc(history_size * sizeof(uint32_t)); if (self->binary_far_history == NULL) { - WebRtc_FreeBinaryDelayEstimator(self); - self = NULL; - return -1; + return_value = -1; } self->binary_near_history = malloc(self->near_history_size * sizeof(uint32_t)); if (self->binary_near_history == NULL) { - WebRtc_FreeBinaryDelayEstimator(self); - self = NULL; - return -1; + return_value = -1; } self->far_bit_counts = malloc(history_size * sizeof(int)); if (self->far_bit_counts == NULL) { - WebRtc_FreeBinaryDelayEstimator(self); - self = NULL; - return -1; + return_value = -1; } - return 0; + if (return_value == -1) { + WebRtc_FreeBinaryDelayEstimator(self); + *handle = NULL; + } + return return_value; } int WebRtc_InitBinaryDelayEstimator(BinaryDelayEstimator* handle) { diff --git a/src/modules/audio_processing/utility/delay_estimator.h b/src/modules/audio_processing/utility/delay_estimator.h index a376dfeb61..e5819b2922 100644 --- a/src/modules/audio_processing/utility/delay_estimator.h +++ b/src/modules/audio_processing/utility/delay_estimator.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011 The WebRTC project authors. All Rights Reserved. + * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved. * * Use of this source code is governed by a BSD-style license * that can be found in the LICENSE file in the root of the source @@ -47,7 +47,7 @@ typedef struct { // Input: // - handle : Pointer to the delay estimation instance. // -int WebRtc_FreeBinaryDelayEstimator(BinaryDelayEstimator* handle); +void WebRtc_FreeBinaryDelayEstimator(BinaryDelayEstimator* handle); // Refer to WebRtc_CreateDelayEstimator() in delay_estimator_wrapper.h. int WebRtc_CreateBinaryDelayEstimator(BinaryDelayEstimator** handle, diff --git a/src/modules/audio_processing/utility/delay_estimator_wrapper.c b/src/modules/audio_processing/utility/delay_estimator_wrapper.c index 438c95f5ec..37a22345e4 100644 --- a/src/modules/audio_processing/utility/delay_estimator_wrapper.c +++ b/src/modules/audio_processing/utility/delay_estimator_wrapper.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011 The WebRTC project authors. All Rights Reserved. + * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved. * * Use of this source code is governed by a BSD-style license * that can be found in the LICENSE file in the root of the source @@ -140,27 +140,23 @@ static uint32_t BinarySpectrumFloat(float* spectrum, return out; } -int WebRtc_FreeDelayEstimator(void* handle) { +void WebRtc_FreeDelayEstimator(void* handle) { DelayEstimator* self = (DelayEstimator*) handle; - if (self == NULL) { - return -1; + if (handle == NULL) { + return; } - if (self->mean_far_spectrum != NULL) { - free(self->mean_far_spectrum); - self->mean_far_spectrum = NULL; - } - if (self->mean_near_spectrum != NULL) { - free(self->mean_near_spectrum); - self->mean_near_spectrum = NULL; - } + free(self->mean_far_spectrum); + self->mean_far_spectrum = NULL; + + free(self->mean_near_spectrum); + self->mean_near_spectrum = NULL; WebRtc_FreeBinaryDelayEstimator(self->binary_handle); + self->binary_handle = NULL; free(self); - - return 0; } int WebRtc_CreateDelayEstimator(void** handle, @@ -168,7 +164,9 @@ int WebRtc_CreateDelayEstimator(void** handle, int max_delay, int lookahead) { DelayEstimator* self = NULL; + int return_value = 0; + // TODO(bjornv): Make this a static assert. // Check if the sub band used in the delay estimation is small enough to fit // the binary spectra in a uint32_t. assert(kBandLast - kBandFirst < 32); @@ -177,6 +175,7 @@ int WebRtc_CreateDelayEstimator(void** handle, return -1; } if (spectrum_size < kBandLast) { + *handle = NULL; return -1; } @@ -193,27 +192,25 @@ int WebRtc_CreateDelayEstimator(void** handle, if (WebRtc_CreateBinaryDelayEstimator(&self->binary_handle, max_delay, lookahead) != 0) { - WebRtc_FreeDelayEstimator(self); - self = NULL; - return -1; + return_value = -1; } // Allocate memory for spectrum buffers. self->mean_far_spectrum = malloc(spectrum_size * sizeof(SpectrumType)); if (self->mean_far_spectrum == NULL) { - WebRtc_FreeDelayEstimator(self); - self = NULL; - return -1; + return_value = -1; } self->mean_near_spectrum = malloc(spectrum_size * sizeof(SpectrumType)); if (self->mean_near_spectrum == NULL) { - WebRtc_FreeDelayEstimator(self); - self = NULL; - return -1; + return_value = -1; } self->spectrum_size = spectrum_size; - return 0; + if (return_value == -1) { + WebRtc_FreeDelayEstimator(self); + *handle = NULL; + } + return return_value; } int WebRtc_InitDelayEstimator(void* handle) { diff --git a/src/modules/audio_processing/utility/delay_estimator_wrapper.h b/src/modules/audio_processing/utility/delay_estimator_wrapper.h index 2a47b5d85a..5d3f0689db 100644 --- a/src/modules/audio_processing/utility/delay_estimator_wrapper.h +++ b/src/modules/audio_processing/utility/delay_estimator_wrapper.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011 The WebRTC project authors. All Rights Reserved. + * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved. * * Use of this source code is governed by a BSD-style license * that can be found in the LICENSE file in the root of the source @@ -20,7 +20,7 @@ // Input: // - handle : Pointer to the delay estimation instance. // -int WebRtc_FreeDelayEstimator(void* handle); +void WebRtc_FreeDelayEstimator(void* handle); // Allocates the memory needed by the delay estimation. The memory needs to be // initialized separately through WebRtc_InitDelayEstimator(...).