[PATCH] D31385: [XRay] Update FDR log reader to be aware of buffer sizes per thread.

Dean Michael Berris via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 28 17:27:49 PDT 2017


dberris added a comment.

LGTM - Few more nits.



================
Comment at: lib/XRay/Trace.cpp:376-377
+  std::memcpy(&BufferSize, &FileHeader.FreeFormData, sizeof(uint64_t));
+  FDRState State{0,          0, 0, FDRState::Token::NEW_BUFFER_RECORD_OR_EOF,
+                 BufferSize, 0};
   // RecordSize will tell the loop how far to seek ahead based on the record
----------------
Did clang-format do this?


================
Comment at: lib/XRay/Trace.cpp:388-390
+            Twine("Incomplete thread buffer. Expected ") +
+                Twine(State.CurrentBufferSize - State.CurrentBufferConsumed) +
+                Twine(" remaining bytes but found ") + Twine(S.size()),
----------------
I think you just need the first object in the '+' concatenation to be a Twine -- the rest can be as they are just fine:

`Twine("Incomplete thread buffer. Expected ") + (State.CurrentBufferSize - State.CurrentBufferConsumed) + " remaining bytes but found " + S.size()`


================
Comment at: lib/XRay/Trace.cpp:416-420
     return make_error<StringError>(
-        "Encountered EOF without preceding End of Buffer record.",
+        Twine("Encountered EOF with unexpected state expectation ") +
+            fdrStateToTwine(State.Expects) +
+            Twine(". Remaining expected bytes in thread buffer total ") +
+            Twine(State.CurrentBufferSize - State.CurrentBufferConsumed),
----------------
Same here.


https://reviews.llvm.org/D31385





More information about the llvm-commits mailing list