Converity 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 830385f03136016da60ab4f8c4264fbdad723b84..ab3ece6e398c151e595cacc240732d97d722e8c2 100644 (file)
--- 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 04759ed2fd809308aaf2a40fc5c95f3759fbac28..7f74b5125b64d50db1c2585a0236fe6c94a71252 100644 (file)
--- 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;