[Lldb-commits] [PATCH] D55214: Introduce ObjectFileBreakpad

[Leonard Mosescu via Phabricator via lldb-commits]

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