[PATCH] Add test showing error in StreamingMemoryObject.setKnownObjectSize().

[Karl Schimpf]

Please take a look at Diff 25053. Thanks.


================
Comment at: include/llvm/Support/StreamingMemoryObject.h:66
@@ -66,1 +65,3 @@
+  // time to avoid making too many potentially expensive GetBytes
+  // calls
   bool fetchToPos(size_t Pos) const {
----------------
jvoung wrote:
> Can you also add a period at the end of the sentence while you are updating this comment?
Done.

================
Comment at: include/llvm/Support/StreamingMemoryObject.h:75
@@ -73,3 +74,3 @@
       BytesRead += bytes;
-      if (bytes != kChunkSize) { // reached EOF/ran out of bytes
-        ObjectSize = BytesRead;
+      if (bytes == 0) { // reached EOF/ran out of bytes
+        if (ObjectSize == 0)
----------------
rafael wrote:
> Why only == 0?
> 
> The previous logic looks correct If we ask for 100 bytes but only get 50, we are at the end of the stream.
> 
I used bytes == 0 following how read works. For the case it is hooked up to a pipe, it may return only some of the requested bytes (not necessarily all). This is done when the pipe doesn't have all the bytes requested. In that case, it returns what it does have (and at least one character). It only returns 0 if you reach the pipe has been closed, and you are at the end of the pipe contents.

This matches what we might expect from a data streamer​. Note that many people misuse read and assume that it will always return the number of requested bytes, unless eof has been reached. It is simply not (always) true. I have been burned too many times and that is why I used bytes == 0.

================
Comment at: include/llvm/Support/StreamingMemoryObject.h:81
@@ -79,3 +80,3 @@
     }
-    return Pos < BytesRead;
+    return true;
   }
----------------
Fixed return value, as  suggested by Rafael.

================
Comment at: lib/Support/StreamingMemoryObject.cpp:91
@@ +90,3 @@
+  size_t MaxAddress =
+      (ObjectSize && ObjectSize < BytesRead) ? ObjectSize : BytesRead;
+  if (Address >= MaxAddress)
----------------
jvoung wrote:
> Maybe add a brief comment about why ObjectSize can be < BytesRead.
Done.

================
Comment at: lib/Support/StreamingMemoryObject.cpp:111
@@ -110,3 @@
-  ObjectSize = size;
-  Bytes.reserve(size);
-}
----------------
jvoung wrote:
> Why not keep the reserve?
Added it back. Also set EOFReached if ObjectSize <= BytesRead.

http://reviews.llvm.org/D8931

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/