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 058fd3fc636f2595aa0313510c18232c9221bb7b..70d4e41a78e17b20b8062ea632ac4cb753ab13fa 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 35ed66b0fb34ea565f42e9b8832e908e77f7b79f..a8090076dc03d1579f9818b644e63e5047ca329b 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;