From: Amos Jeffries <amosjeffries@squid-cache.org>
Date: Mon, 6 Oct 2008 11:35:50 +0000 (-0600)
Subject: Coverity detected. inconsistent error detection on StoreIOBUffer
X-Git-Tag: SQUID_3_0_STABLE10~9
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=16f1d46d80c63d17a92d9e37e6bd432a148c3f31;p=thirdparty%2Fsquid.git

Coverity detected. inconsistent error detection on StoreIOBUffer

Audit reveals only one of several callers which might set negative length
state were checking for it and setting error flag properly.

Makes more sense for the StoreIOBuffer constructor to do its own
error state detection with information than to offload on callers.
---

diff --git a/src/StoreIOBuffer.h b/src/StoreIOBuffer.h
index 058fd3fc63..70d4e41a78 100644
--- a/src/StoreIOBuffer.h
+++ b/src/StoreIOBuffer.h
@@ -43,12 +43,20 @@ class StoreIOBuffer
 {
 
 public:
-    StoreIOBuffer():length(0), offset (0), data (NULL){flags.error = 0;}
+    StoreIOBuffer():length(0), offset (0), data (NULL) {flags.error = 0;}
 
     StoreIOBuffer(size_t aLength, int64_t anOffset, char *someData) :
-            length (aLength), offset (anOffset), data (someData)
+            offset (anOffset), data (someData)
     {
-        flags.error = 0;
+        /* maintain own state: detect size errors now */
+        if (aLength <0) {
+            flags.error = 1;
+            length = 0;
+        }
+        else {
+            flags.error = 0;
+            length = aLength;
+        }
     }
 
     /* Create a StoreIOBuffer from a MemBuf and offset */
diff --git a/src/store_client.cc b/src/store_client.cc
index 35ed66b0fb..a8090076dc 100644
--- a/src/store_client.cc
+++ b/src/store_client.cc
@@ -143,13 +143,10 @@ storeClientListAdd(StoreEntry * e, void *data)
 void
 store_client::callback(ssize_t sz, bool error)
 {
-    StoreIOBuffer result (sz, 0 ,copyInto.data);
+    StoreIOBuffer result(sz, 0 ,copyInto.data);
 
-    if (sz < 0) {
+    if (error) {
         result.flags.error = 1;
-        result.length = 0;
-    } else {
-        result.flags.error = error ? 1 : 0;
     }
 
     result.offset = cmp_offset;