Further code review feedback and formatting.

Author: Jlalond

lldb/include/lldb/API/SBCoreDumpOptions.h

@@ -22,7 +22,8 @@ class LLDB_API SBCoreDumpOptions {
 
   const SBCoreDumpOptions &operator=(const lldb::SBCoreDumpOptions &rhs);
 
-  /// Set the plugin name.
+  /// Set the plugin name. Supplying null or empty string will reset
+  /// the option.
   ///
   /// \param plugin Name of the object file plugin.
   SBError SetPluginName(const char *plugin);

lldb/include/lldb/Core/PluginManager.h

@@ -178,7 +178,7 @@ class PluginManager {
 
   static bool UnregisterPlugin(ObjectFileCreateInstance create_callback);
 
-  static bool IsRegisteredPluginName(const char *name);
+  static bool IsRegisteredObjectFilePluginName(llvm::StringRef name);
 
   static ObjectFileCreateInstance
   GetObjectFileCreateCallbackAtIndex(uint32_t idx);

lldb/source/API/SBCoreDumpOptions.cpp

@@ -80,7 +80,7 @@ lldb_private::CoreDumpOptions &SBCoreDumpOptions::ref() const {
   return *m_opaque_up.get();
 }
 
-void SBCoreDumpOptions::Clear() { 
-    LLDB_INSTRUMENT_VA(this);
-    m_opaque_up->Clear(); 
+void SBCoreDumpOptions::Clear() {
+  LLDB_INSTRUMENT_VA(this);
+  m_opaque_up->Clear();
 }

lldb/source/Commands/CommandObjectProcess.cpp

@@ -1256,7 +1256,7 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed {
 
   class CommandOptions : public Options {
   public:
-    CommandOptions(){};
+    CommandOptions() = default;
 
     ~CommandOptions() override = default;
 

lldb/source/Core/PluginManager.cpp

@@ -640,8 +640,8 @@ static ObjectFileInstances &GetObjectFileInstances() {
   return g_instances;
 }
 
-bool PluginManager::IsRegisteredPluginName(const char *name) {
-  if (!name || !name[0])
+bool PluginManager::IsRegisteredObjectFilePluginName(llvm::StringRef name) {
+  if (name.empty())
     return false;
 
   const auto &instances = GetObjectFileInstances().GetInstances();

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

@@ -6524,11 +6524,10 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
   auto core_style = options.GetStyle();
   if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
     core_style = SaveCoreStyle::eSaveCoreDirtyOnly;
-  // The FileSpec is already checked in PluginManager::SaveCore.
+  // The FileSpec and Process are already checked in PluginManager::SaveCore.
   assert(options.GetOutputFile().has_value());
   const FileSpec outfile = options.GetOutputFile().value();
-  if (!process_sp)
-    return false;
+  assert(process_sp);
 
   Target &target = process_sp->GetTarget();
   const ArchSpec target_arch = target.GetArchitecture();

lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp

@@ -58,9 +58,9 @@ size_t ObjectFileMinidump::GetModuleSpecifications(
 bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
                                   const lldb_private::CoreDumpOptions &options,
                                   lldb_private::Status &error) {
+  // Output file and process_sp are both checked in PluginManager::SaveCore.
   assert(options.GetOutputFile().has_value());
-  if (!process_sp)
-    return false;
+  assert(process_sp);
 
   // Minidump defaults to stacks only.
   SaveCoreStyle core_style = options.GetStyle();

lldb/source/Symbol/CoreDumpOptions.cpp

@@ -16,12 +16,11 @@ Status CoreDumpOptions::SetPluginName(const char *name) {
   Status error;
   if (!name || !name[0]) {
     m_plugin_name = std::nullopt;
-    error.SetErrorString("no plugin name specified");
   }
 
-  if (!PluginManager::IsRegisteredPluginName(name)) {
+  if (!PluginManager::IsRegisteredObjectFilePluginName(name)) {
     error.SetErrorStringWithFormat(
-        "plugin name '%s' is not a registered plugin", name);
+        "plugin name '%s' is not a valid ObjectFile plugin name", name);
     return error;
   }
 
@@ -38,9 +37,7 @@ std::optional<std::string> CoreDumpOptions::GetPluginName() const {
 }
 
 lldb::SaveCoreStyle CoreDumpOptions::GetStyle() const {
-  if (!m_style.has_value())
-    return lldb::eSaveCoreUnspecified;
-  return m_style.value();
+  return m_style.value_or(lldb::eSaveCoreUnspecified);
 }
 
 const std::optional<lldb_private::FileSpec>

lldb/test/API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py

@@ -10,14 +10,17 @@ def test_plugin_name_assignment(self):
         """Test assignment ensuring valid plugin names only."""
         options = lldb.SBCoreDumpOptions()
         error = options.SetPluginName(None)
-        self.assertTrue(error.Fail())
+        self.assertTrue(error.Success())
         self.assertEqual(options.GetPluginName(), None)
         error = options.SetPluginName("Not a real plugin")
-        self.assertTrue(error.Fail())
+        self.assertTrue(error.Success())
         self.assertEqual(options.GetPluginName(), None)
         error = options.SetPluginName("minidump")
         self.assertTrue(error.Success())
         self.assertEqual(options.GetPluginName(), "minidump")
+        error = options.SetPluginName("")
+        self.assertTrue(error.Success())
+        self.assertEqual(options.GetPluginName(), None)
 
     def test_default_corestyle_behavior(self):
         """Test that the default core style is unspecified."""