Address earlier code review comments

Change-Id: I4d369ba153676d0990dadf9b1269506607adec7f

diff --git a/audio_utils/fifo.cpp b/audio_utils/fifo.cpp
index 958bbfb..31c614d 100644 (file)
--- a/audio_utils/fifo.cpp
+++ b/audio_utils/fifo.cpp
@@ -17,6 +17,7 @@
 //#define LOG_NDEBUG 0
 #define LOG_TAG "audio_utils_fifo"
 
+#include <limits.h>
 #include <stdlib.h>
 #include <string.h>
 #include <audio_utils/fifo.h>
@@ -31,7 +32,7 @@ audio_utils_fifo_base::audio_utils_fifo_base(uint32_t frameCount)
     mSharedRear(0), mThrottleFront(NULL)
 {
     // actual upper bound on frameCount will depend on the frame size
-    ALOG_ASSERT(frameCount > 0 && frameCount <= ((uint32_t) INT_MAX));
+    LOG_ALWAYS_FATAL_IF(frameCount == 0 || frameCount > ((uint32_t) INT_MAX));
 }
 
 audio_utils_fifo_base::~audio_utils_fifo_base()
@@ -97,8 +98,8 @@ audio_utils_fifo::audio_utils_fifo(uint32_t frameCount, uint32_t frameSize, void
 {
     // maximum value of frameCount * frameSize is INT_MAX (2^31 - 1), not 2^31, because we need to
     // be able to distinguish successful and error return values from read and write.
-    ALOG_ASSERT(frameCount > 0 && frameSize > 0 && buffer != NULL &&
-            frameCount <= ((uint32_t) INT_MAX) / frameSize);
+    LOG_ALWAYS_FATAL_IF(frameCount == 0 || frameSize == 0 || buffer == NULL ||
+            frameCount > ((uint32_t) INT_MAX) / frameSize);
 }
 
 audio_utils_fifo::~audio_utils_fifo()
@@ -182,7 +183,7 @@ void audio_utils_fifo_writer::release(size_t count)
         __attribute__((no_sanitize("integer")))
 {
     if (count > 0) {
-        ALOG_ASSERT(count <= mObtained);
+        LOG_ALWAYS_FATAL_IF(count > mObtained);
         mLocalRear = mFifo.sum(mLocalRear, count);
         atomic_store_explicit(&mFifo.mSharedRear, (uint_fast32_t) mLocalRear,
                 std::memory_order_release);
@@ -196,13 +197,14 @@ audio_utils_fifo_reader::audio_utils_fifo_reader(audio_utils_fifo& fifo, bool th
     audio_utils_fifo_provider(), mFifo(fifo), mLocalFront(0), mSharedFront(0)
 {
     if (throttlesWriter) {
-        ALOG_ASSERT(fifo.mThrottleFront == NULL);
+        LOG_ALWAYS_FATAL_IF(fifo.mThrottleFront != NULL);
         fifo.mThrottleFront = &mSharedFront;
     }
 }
 
 audio_utils_fifo_reader::~audio_utils_fifo_reader()
 {
+    // TODO Need a way to pass throttle capability to the another reader, should one reader exit.
     if (mFifo.mThrottleFront == &mSharedFront) {
         mFifo.mThrottleFront = NULL;
     }
@@ -236,7 +238,7 @@ void audio_utils_fifo_reader::release(size_t count)
         __attribute__((no_sanitize("integer")))
 {
     if (count > 0) {
-        ALOG_ASSERT(count <= mObtained);
+        LOG_ALWAYS_FATAL_IF(count > mObtained);
         mLocalFront = mFifo.sum(mLocalFront, count);
         if (mFifo.mThrottleFront == &mSharedFront) {
             atomic_store_explicit(&mSharedFront, (uint_fast32_t) mLocalFront,

diff --git a/audio_utils/include/audio_utils/fifo.h b/audio_utils/include/audio_utils/fifo.h
index 56d7bbb..1688dea 100644 (file)
--- a/audio_utils/include/audio_utils/fifo.h
+++ b/audio_utils/include/audio_utils/fifo.h
@@ -31,7 +31,7 @@ class audio_utils_fifo_base {
 
 protected:
     audio_utils_fifo_base(uint32_t frameCount);
-    virtual ~audio_utils_fifo_base();
+    /*virtual*/ ~audio_utils_fifo_base();
 
 /** Return a new index as the sum of a validated index and a specified increment.
  *
@@ -89,7 +89,7 @@ public:
  */
     audio_utils_fifo(uint32_t frameCount, uint32_t frameSize, void *buffer);
 
-    virtual ~audio_utils_fifo();
+    /*virtual*/ ~audio_utils_fifo();
 
 private: