[PATCH] D34339: [XRay] fix and clarify comments in the log file decoder

Dean Michael Berris via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 19 00:10:46 PDT 2017


dberris accepted this revision.
dberris added a comment.
This revision is now accepted and ready to land.

Some nits.



================
Comment at: lib/XRay/Trace.cpp:62
     return make_error<StringError>(
-        "Not enough bytes for an XRay log.",
+        "Not enough bytes for an XRay log header.",
         std::make_error_code(std::errc::invalid_argument));
----------------
The error was already correct -- there's not enough data for an XRay log.


================
Comment at: lib/XRay/Trace.cpp:225
   // We can encounter a CustomEventMarker anywhere in the log, so we can handle
-  // it regardless of the expectation. However, we do se the expectation to read
-  // a set number of fixed bytes, as described in the metadata.
+  // it regardless of the expectation. However, we do see the expectation to
+  // read a set number of fixed bytes, as described in the metadata.
----------------
see -> set


================
Comment at: lib/XRay/Trace.cpp:327
     default:
-      // When initializing the error, convert to uint16_t so that the record
+      // When initializing the error, convert to an integer so that the record
       // type isn't interpreted as a char.
----------------
unsigned integer?


================
Comment at: lib/XRay/Trace.cpp:441-442
+    State.CurrentBufferConsumed += RecordSize;
   }
-  // There are two conditions
-  if (State.Expects != FDRState::Token::NEW_BUFFER_RECORD_OR_EOF &&
-      !(State.Expects == FDRState::Token::SCAN_TO_END_OF_THREAD_BUF &&
-        State.CurrentBufferSize == State.CurrentBufferConsumed))
+  // Having iterated over everything we've been given, we've either consumed
+  // everything and ended up in the end state, or were told to skip the rest.
----------------
Empty line between non-comment line and first comment line?


https://reviews.llvm.org/D34339





More information about the llvm-commits mailing list