[Lldb-commits] [PATCH] D125434: Make a more convenient way to allow Darwin users to ignore certain Mach Exceptions

[Jonas Devlieghere via Phabricator via lldb-commits]

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