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