[Lldb-commits] [PATCH] D88632: A handful of fixes to lldb's "scan the local filesystem for kexts & kernels" feature

Jonas Devlieghere via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu Oct 1 09:21:03 PDT 2020

JDevlieghere added a comment.

I don't know much about this code so I've left some inline nits.

Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp:505
+  bool is_kernel_filename =
+      file_spec.GetFilename().GetStringRef().startswith("kernel") ||
+      file_spec.GetFilename().GetStringRef().startswith("mach");
Nit: You could assign the StringRef to a temporary so you don't have to repeat `file_spec.GetFilename().GetStringRef()` thrice. 

Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp:680
+  std::string possible_path = kernel_dsym.GetPath();
+  if (!kernel_dsym.GetFilename().GetStringRef().endswith(".dSYM"))
+    return false;
`FileSpec` has a `GetFileNameExtension` but this works too. 

Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp:684-685
+  FileSpec binary_filespec = kernel_dsym;
+  std::string filename = binary_filespec.GetFilename().GetCString();
+  filename.erase(filename.size() - 5, filename.size()); // chop off '.dSYM'
+  binary_filespec.GetFilename() = ConstString(filename);

Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp:716
+  // get rid of '.dSYM'
+  filename.erase(filename.size() - 5, filename.size());

Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp:865
+        } else {
+          error = ModuleList::GetSharedModule(kern_spec, module_sp, NULL, NULL,
+                                              NULL);
Huh, how did we end up with `NULL` again, someone ran a clang tidy check over lldb that replaces it with `nullptr`. Do we only build this on Darwin maybe? If the rest of the file uses `NULL` we can fix that inconsistency in a separate commit. 

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list