JDevlieghere added inline comments.
================ Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:80-111 +// Static Variables +static uint32_t g_initialize_count = 0; + +void PlatformDarwin::Initialize() { + Platform::Initialize(); + + if (g_initialize_count++ == 0) { ---------------- What's the point of having this? Isn't `PlatformDarwin` an abstract class anyway? ================ Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:166 + debugger, GetGlobalProperties().GetValueProperties(), + ConstString("Properties for the PlatformDarwin plug-in."), + is_global_setting); ---------------- `PlatformDarwin` is the name of the class, but I don't think we ever present it that way to the user. ================ Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:176-177 + const char *ignored_exceptions = GetGlobalProperties().GetIgnoredExceptions(); + if (!ignored_exceptions || ignored_exceptions[0] == '\0' ) + return {}; + Args ret_args; ---------------- Unless you expect this string to contain a null byte you could turn this into a StringRef and check `::empty()` instead. ================ Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:959-964 + size_t num_cmds = args.GetArgumentCount(); + for (size_t idx = 0; idx < num_cmds; idx++) { + StringExtractorGDBRemote response; + m_gdb_comm.SendPacketAndWaitForResponse( + args.GetArgumentAtIndex(idx), response); + } ---------------- You can use a range-based for loop here. ================ Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:967-970 + PlatformSP platform_sp = GetTarget().GetPlatform(); + if (platform_sp) { + handle_cmds(platform_sp->GetExtraStartupCommands()); } ---------------- nit: LLVM likes to do this but I personally don't care. ================ Comment at: lldb/tools/debugserver/source/MacOSX/MachException.cpp:527-558 + if (strcmp(name, "BAD_ACCESS") == 0) + return EXC_MASK_BAD_ACCESS; + if (strcmp(name, "BAD_INSTRUCTION") == 0) + return EXC_MASK_BAD_INSTRUCTION; + if (strcmp(name, "ARITHMETIC") == 0) + return EXC_MASK_ARITHMETIC; + if (strcmp(name, "EMULATION") == 0) ---------------- Why not make it a string and use it's equals method? All these `strcmp`s make the whole this unnecessary verbose imho. ```if (name == "BAD_ACCESS")``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125434/new/ https://reviews.llvm.org/D125434 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits