lemo accepted this revision.
lemo added a comment.

Looks like a great start



================
Comment at: source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp:53
+  static_assert(sizeof(data) == 20, "");
+  if (str.size() < 33 || str.size() > 40)
+    return UUID();
----------------
these magic integer literals make it hard to follow the intent - what's special 
about 33, 40, 8, 16, ... ? (symbolic constants might help)


================
Comment at: source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h:98
+
+private:
+  struct Header {
----------------
Nit: I personally prefer not to mix data, type and function members in the same 
"access" section - is there an LLVM/LLDB guideline which requires everything in 
the same place?

If not, can you please add a private section for the destructor, followed by a 
section for the private data members?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55214/new/

https://reviews.llvm.org/D55214



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to