SharedBuffer: Fix bug in return value of release()

Since the equality operator '==' has higher precedence than the
assignment operator '=', we were assigning 'prev' to the result of
our comparison and not the result of mRefs.fetch_sub().

This means that 'prev' would only receive the values 0 or 1.  In
the cases where fetch_sub() returned 0 or 1, we were happening to
get the correct value.  But if fetch_sub() was greator than 1,
we would return to the user 0, instead of the previous reference
count.

We fix this by properly adding parentheses.  We also adjust the
whitespace a little to hopefully make the groupings of the logic
easier to see.

Change-Id: Ib129798a7076854b9ca4f6385c42edbf4fb75e57

1 files changed, 3 insertions, 2 deletions

diff --git a/libutils/SharedBuffer.cpp b/libutils/SharedBuffer.cpp
index a8a9fb415..6c8c7d3dc 100644
--- a/libutils/SharedBuffer.cpp
+++ b/libutils/SharedBuffer.cpp
@@ -112,8 +112,9 @@ void SharedBuffer::acquire() const {
 int32_t SharedBuffer::release(uint32_t flags) const
 {
     int32_t prev = 1;
-    if (onlyOwner() || ((prev = mRefs.fetch_sub(1, std::memory_order_release) == 1)
-            && (atomic_thread_fence(std::memory_order_acquire), true))) {
+    if (onlyOwner()
+            || (((prev = mRefs.fetch_sub(1, std::memory_order_release)) == 1)
+                && (atomic_thread_fence(std::memory_order_acquire), true))) {
         mRefs.store(0, std::memory_order_relaxed);
         if ((flags & eKeepStorage) == 0) {
             free(const_cast<SharedBuffer*>(this));