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

Jan Voung jvoung at chromium.org
Wed Apr 15 10:42:56 PDT 2015


Thanks -- looks okay to me, but wait for Rafael to take a look too since he some earlier questions?


================
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 {
----------------
Can you also add a period at the end of the sentence while you are updating this comment?

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

================
Comment at: lib/Support/StreamingMemoryObject.cpp:111
@@ -110,3 @@
-  ObjectSize = size;
-  Bytes.reserve(size);
-}
----------------
Why not keep the reserve?

http://reviews.llvm.org/D8931

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






More information about the llvm-commits mailing list