A little bit more pointer safety in the Breakpoint code

[zturner]

Summary

This started out when I noticed a potential pitfall in the API of SetCallback where you could potentially get the class into a bad state. Ultimately I wasn't able to fix that in this patch, although I did include a comment in this patch explaining the problem and some possible fixes. But my initial attempts to fix it before I decided that the best approach wasn't clear ended up in this patch, which I think is still a net positive, so I'm uploading it anyway.

Basically, it makes the Baton classes a little bit more type-safe by plumbing unique_ptrs through a few more places so that you never have to deal with raw pointers, and it also makes it so Batons can clean up after themselves by default, so anyone implementing a new baton doesn't have to reimplement the exact same code in the destructor every time.

Also fixes a compiler error since SIZE_T_MAX isn't defined.

[zturner]

There were a few outstanding compiler errors that I missed in my previous patch as I was doing it on a build without Python. This should catch all of them.

[jingham]

Looks fine. The only bad effect of making a CommandBaton, then going out of your way to put it in a plain old BatonSP before passing it to us rather than a CommandBatonSP is that the serialization code won't know it is a command baton, and won't serialize it. We use the llvm isa stuff in a bunch of places so if this becomes a problem it would be natural to fix it that way.

lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h

private:
 
     static const char *g_option_names[LastOptionName];
 
     static const char *GetKey(enum OptionNames enum_value) {
       return g_option_names[enum_value];
     }
   };
 
-  class CommandBaton : public Baton {
+  class CommandBaton : public TypedBaton<CommandData> {
   public:
-    CommandBaton(CommandData *data) : Baton(data) {}
-
-    ~CommandBaton() override {
-      delete ((CommandData *)m_data);
-      m_data = nullptr;
-    }
+    explicit CommandBaton(std::unique_ptr<CommandData> Data)
+        : TypedBaton(std::move(Data)) {}
 
     void GetDescription(Stream *s, lldb::DescriptionLevel level) const override;
   };
 
   typedef std::shared_ptr<CommandBaton> CommandBatonSP;
 
   //------------------------------------------------------------------
   // Constructors and Destructors
static bool NullCallback(void *baton, StoppointCallbackContext *context,
                            lldb::user_id_t break_loc_id);
 
   //------------------------------------------------------------------
   /// Set a callback based on BreakpointOptions::CommandData.
   /// @param[in] cmd_data
   ///     A new'ed CommandData object.  The breakpoint will take ownership
   ///     of this object.
   //------------------------------------------------------------------
-  void SetCommandDataCallback(CommandData *cmd_data);
+  void SetCommandDataCallback(std::unique_ptr<CommandData> cmd_data);
 
 protected:
   //------------------------------------------------------------------
   // Classes that inherit from BreakpointOptions can see and modify these
   //------------------------------------------------------------------
   enum OptionNames {
     ConditionText = 0,
     IgnoreCount,

lldb/trunk/include/lldb/Breakpoint/WatchpointOptions.h

struct CommandData {
 
     ~CommandData() = default;
 
     StringList user_source;
     std::string script_source;
     bool stop_on_error;
   };
 
-  class CommandBaton : public Baton {
+  class CommandBaton : public TypedBaton<CommandData> {
   public:
-    CommandBaton(CommandData *data) : Baton(data) {}
-
-    ~CommandBaton() override {
-      delete ((CommandData *)m_data);
-      m_data = nullptr;
-    }
+    CommandBaton(std::unique_ptr<CommandData> Data)
+        : TypedBaton(std::move(Data)) {}
 
     void GetDescription(Stream *s, lldb::DescriptionLevel level) const override;
   };
 
 protected:
   //------------------------------------------------------------------
   // Classes that inherit from WatchpointOptions can see and modify these
   //------------------------------------------------------------------

lldb/trunk/include/lldb/Core/Baton.h

 /// callback function that takes a baton where the baton might need to
 /// free/delete/close itself.
 ///
 /// The default behavior is to not free anything. Subclasses can
 /// free any needed resources in their destructors.
 //----------------------------------------------------------------------
 class Baton {
 public:
-  explicit Baton(void *p) : m_data(p) {}
+  Baton() {}
+  virtual ~Baton() {}
 
-  virtual ~Baton() {
-    // The default destructor for a baton does NOT attempt to clean up
-    // anything in m_baton
+  virtual void *data() = 0;
+
+  virtual void GetDescription(Stream *s,
+                              lldb::DescriptionLevel level) const = 0;
+};
+
+class UntypedBaton : public Baton {
+public:
+  UntypedBaton(void *Data) : m_data(Data) {}
+  virtual ~UntypedBaton() {
+    // The default destructor for an untyped baton does NOT attempt to clean up
+    // anything in m_data.
   }
 
-  virtual void GetDescription(Stream *s, lldb::DescriptionLevel level) const;
+  void *data() override { return m_data; }
+  void GetDescription(Stream *s, lldb::DescriptionLevel level) const override;
 
   void *m_data; // Leave baton public for easy access
+};
+
+template <typename T> class TypedBaton : public Baton {
+public:
+  explicit TypedBaton(std::unique_ptr<T> Item) : Item(std::move(Item)) {}
+
+  T *getItem() { return Item.get(); }
+  const T *getItem() const { return Item.get(); }
+
+  void *data() override { return Item.get(); }
+  virtual void GetDescription(Stream *s,
+                              lldb::DescriptionLevel level) const override {}
 
-private:
-  //------------------------------------------------------------------
-  // For Baton only
-  //------------------------------------------------------------------
-  DISALLOW_COPY_AND_ASSIGN(Baton);
+protected:
+  std::unique_ptr<T> Item;
 };
 
 } // namespace lldb_private
 
 #endif // lldb_Baton_h_

lldb/trunk/source/API/SBBreakpoint.cpp

 #include "lldb/Target/Process.h"
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadSpec.h"
 
 #include "lldb/lldb-enumerations.h"
 
+#include "llvm/ADT/STLExtras.h"
+
 using namespace lldb;
 using namespace lldb_private;
 
 struct CallbackData {
   SBBreakpoint::BreakpointHitCallback callback;
   void *callback_baton;
 };
 
-class SBBreakpointCallbackBaton : public Baton {
+class SBBreakpointCallbackBaton : public TypedBaton<CallbackData> {
 public:
   SBBreakpointCallbackBaton(SBBreakpoint::BreakpointHitCallback callback,
                             void *baton)
-      : Baton(new CallbackData) {
-    CallbackData *data = (CallbackData *)m_data;
-    data->callback = callback;
-    data->callback_baton = baton;
-  }
-
-  ~SBBreakpointCallbackBaton() override {
-    CallbackData *data = (CallbackData *)m_data;
-
-    if (data) {
-      delete data;
-      m_data = nullptr;
-    }
+      : TypedBaton(llvm::make_unique<CallbackData>()) {
+    getItem()->callback = callback;
+    getItem()->callback_baton = baton;
   }
 };
 
 SBBreakpoint::SBBreakpoint() : m_opaque_sp() {}
 
 SBBreakpoint::SBBreakpoint(const SBBreakpoint &rhs)
     : m_opaque_sp(rhs.m_opaque_sp) {}
 

lldb/trunk/source/Breakpoint/Breakpoint.cpp

const char *Breakpoint::GetConditionText() const {
   return m_options_up->GetConditionText();
 }
 
 // This function is used when "baton" doesn't need to be freed
 void Breakpoint::SetCallback(BreakpointHitCallback callback, void *baton,
                              bool is_synchronous) {
   // The default "Baton" class will keep a copy of "baton" and won't free
   // or delete it when it goes goes out of scope.
-  m_options_up->SetCallback(callback, BatonSP(new Baton(baton)),
+  m_options_up->SetCallback(callback, std::make_shared<UntypedBaton>(baton),
                             is_synchronous);
 
   SendBreakpointChangedEvent(eBreakpointEventTypeCommandChanged);
 }
 
 // This function is used when a baton needs to be freed and therefore is
 // contained in a "Baton" subclass.
 void Breakpoint::SetCallback(BreakpointHitCallback callback,

lldb/trunk/source/Breakpoint/BreakpointLocation.cpp

bool BreakpointLocation::InvokeCallback(StoppointCallbackContext *context) {
   else
     return m_owner.InvokeCallback(context, GetID());
 }
 
 void BreakpointLocation::SetCallback(BreakpointHitCallback callback,
                                      void *baton, bool is_synchronous) {
   // The default "Baton" class will keep a copy of "baton" and won't free
   // or delete it when it goes goes out of scope.
-  GetLocationOptions()->SetCallback(callback, BatonSP(new Baton(baton)),
-                                    is_synchronous);
+  GetLocationOptions()->SetCallback(
+      callback, std::make_shared<UntypedBaton>(baton), is_synchronous);
   SendBreakpointLocationChangedEvent(eBreakpointEventTypeCommandChanged);
 }
 
 void BreakpointLocation::SetCallback(BreakpointHitCallback callback,
                                      const BatonSP &baton_sp,
                                      bool is_synchronous) {
   GetLocationOptions()->SetCallback(callback, baton_sp, is_synchronous);
   SendBreakpointLocationChangedEvent(eBreakpointEventTypeCommandChanged);

lldb/trunk/source/Breakpoint/BreakpointOptions.cpp

 #include "lldb/Core/StringList.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/ThreadSpec.h"
 
+#include "llvm/ADT/STLExtras.h"
+
 using namespace lldb;
 using namespace lldb_private;
 
 const char *BreakpointOptions::CommandData::g_option_names
     [BreakpointOptions::CommandData::OptionNames::LastOptionName]{
         "UserSource", "ScriptSource", "StopOnError"};
 
 StructuredData::ObjectSP
if (success && cmds_dict) {
     if (cmds_error.Fail()) {
       error.SetErrorStringWithFormat(
           "Failed to deserialize breakpoint command options: %s.",
           cmds_error.AsCString());
       return nullptr;
     }
   }
 
-  BreakpointOptions *bp_options = new BreakpointOptions(
+  auto bp_options = llvm::make_unique<BreakpointOptions>(
       condition_text.c_str(), enabled, ignore_count, one_shot);
-  if (cmd_data)
-    bp_options->SetCommandDataCallback(cmd_data.release());
-  return std::unique_ptr<BreakpointOptions>(bp_options);
+  bp_options->SetCommandDataCallback(std::move(cmd_data));
+  return bp_options;
 }
 
 StructuredData::ObjectSP BreakpointOptions::SerializeToStructuredData() {
   StructuredData::DictionarySP options_dict_sp(
       new StructuredData::Dictionary());
   options_dict_sp->AddBooleanItem(GetKey(OptionNames::EnabledState), m_enabled);
   options_dict_sp->AddBooleanItem(GetKey(OptionNames::OneShotState),
                                   m_one_shot);
   options_dict_sp->AddIntegerItem(GetKey(OptionNames::IgnoreCount),
                                   m_ignore_count);
   options_dict_sp->AddStringItem(GetKey(OptionNames::ConditionText),
                                  m_condition_text);
   if (m_baton_is_command_baton) {
-    CommandData *cmd_data =
-        static_cast<CommandData *>(m_callback_baton_sp->m_data);
+    auto cmd_baton =
+        std::static_pointer_cast<CommandBaton>(m_callback_baton_sp);
     StructuredData::ObjectSP commands_sp =
-        cmd_data->SerializeToStructuredData();
+        cmd_baton->getItem()->SerializeToStructuredData();
     if (commands_sp) {
       options_dict_sp->AddItem(
           BreakpointOptions::CommandData::GetSerializationKey(), commands_sp);
     }
   }
   // FIXME: Need to serialize thread filter...
   return options_dict_sp;
 }
 
 //------------------------------------------------------------------
 // Callbacks
 //------------------------------------------------------------------
 void BreakpointOptions::SetCallback(BreakpointHitCallback callback,
                                     const lldb::BatonSP &callback_baton_sp,
                                     bool callback_is_synchronous) {
+  // FIXME: This seems unsafe.  If BatonSP actually *is* a CommandBaton, but
+  // in a shared_ptr<Baton> instead of a shared_ptr<CommandBaton>, then we
+  // will set m_baton_is_command_baton to false, which is incorrect.
+  // One possible solution is to make the base Baton class provide a method
+  // such as:
+  //     virtual StringRef getBatonId() const { return ""; }
+  // and have CommandBaton override this to return something unique, and then
+  // check for it here.  Another option might be to make Baton using the llvm
+  // casting infrastructure, so that we could write something like:
+  //     if (llvm::isa<CommandBaton>(callback_baton_sp))
+  // at relevant callsites instead of storing a boolean.
   m_callback_is_synchronous = callback_is_synchronous;
   m_callback = callback;
   m_callback_baton_sp = callback_baton_sp;
   m_baton_is_command_baton = false;
 }
 
 void BreakpointOptions::SetCallback(
     BreakpointHitCallback callback,

 const Baton *BreakpointOptions::GetBaton() const {
   return m_callback_baton_sp.get();
 }
 
 bool BreakpointOptions::InvokeCallback(StoppointCallbackContext *context,
                                        lldb::user_id_t break_id,
                                        lldb::user_id_t break_loc_id) {
   if (m_callback && context->is_synchronous == IsCallbackSynchronous()) {
-    return m_callback(m_callback_baton_sp ? m_callback_baton_sp->m_data
+    return m_callback(m_callback_baton_sp ? m_callback_baton_sp->data()
                                           : nullptr,
                       context, break_id, break_loc_id);
   } else
     return true;
 }
 
 bool BreakpointOptions::HasCallback() const {
   return m_callback != BreakpointOptions::NullCallback;
if (level != eDescriptionLevelBrief) {
       s->EOL();
       s->Printf("Condition: %s\n", m_condition_text.c_str());
     }
   }
 }
 
 void BreakpointOptions::CommandBaton::GetDescription(
     Stream *s, lldb::DescriptionLevel level) const {
-  CommandData *data = (CommandData *)m_data;
+  const CommandData *data = getItem();
 
   if (level == eDescriptionLevelBrief) {
     s->Printf(", commands = %s",
               (data && data->user_source.GetSize() > 0) ? "yes" : "no");
     return;
   }
 
   s->IndentMore();
   s->Indent("Breakpoint commands:\n");
 
   s->IndentMore();
   if (data && data->user_source.GetSize() > 0) {
     const size_t num_strings = data->user_source.GetSize();
     for (size_t i = 0; i < num_strings; ++i) {
       s->Indent(data->user_source.GetStringAtIndex(i));
       s->EOL();
     }
   } else {
     s->PutCString("No commands.\n");
   }
   s->IndentLess();
   s->IndentLess();
 }
 
-void BreakpointOptions::SetCommandDataCallback(CommandData *cmd_data) {
-  CommandBatonSP baton_sp(new CommandBaton(cmd_data));
+void BreakpointOptions::SetCommandDataCallback(
+    std::unique_ptr<CommandData> cmd_data) {
+  auto baton_sp = std::make_shared<CommandBaton>(std::move(cmd_data));
   SetCallback(BreakpointOptions::BreakpointOptionsCallbackFunction, baton_sp);
 }
 
 bool BreakpointOptions::BreakpointOptionsCallbackFunction(
     void *baton, StoppointCallbackContext *context, lldb::user_id_t break_id,
     lldb::user_id_t break_loc_id) {
   bool ret_value = true;
   if (baton == nullptr)

lldb/trunk/source/Breakpoint/Watchpoint.cpp

 
 Watchpoint::~Watchpoint() = default;
 
 // This function is used when "baton" doesn't need to be freed
 void Watchpoint::SetCallback(WatchpointHitCallback callback, void *baton,
                              bool is_synchronous) {
   // The default "Baton" class will keep a copy of "baton" and won't free
   // or delete it when it goes goes out of scope.
-  m_options.SetCallback(callback, BatonSP(new Baton(baton)), is_synchronous);
+  m_options.SetCallback(callback, std::make_shared<UntypedBaton>(baton),
+                        is_synchronous);
 
   SendWatchpointChangedEvent(eWatchpointEventTypeCommandChanged);
 }
 
 // This function is used when a baton needs to be freed and therefore is
 // contained in a "Baton" subclass.
 void Watchpoint::SetCallback(WatchpointHitCallback callback,
                              const BatonSP &callback_baton_sp,

lldb/trunk/source/Breakpoint/WatchpointOptions.cpp

 
 const Baton *WatchpointOptions::GetBaton() const {
   return m_callback_baton_sp.get();
 }
 
 bool WatchpointOptions::InvokeCallback(StoppointCallbackContext *context,
                                        lldb::user_id_t watch_id) {
   if (m_callback && context->is_synchronous == IsCallbackSynchronous()) {
-    return m_callback(m_callback_baton_sp ? m_callback_baton_sp->m_data
+    return m_callback(m_callback_baton_sp ? m_callback_baton_sp->data()
                                           : nullptr,
                       context, watch_id);
   } else
     return true;
 }
 
 bool WatchpointOptions::HasCallback() {
   return m_callback != WatchpointOptions::NullCallback;
if ((GetThreadSpecNoCreate() != nullptr &&
     }
   }
 
   GetCallbackDescription(s, level);
 }
 
 void WatchpointOptions::CommandBaton::GetDescription(
     Stream *s, lldb::DescriptionLevel level) const {
-  CommandData *data = (CommandData *)m_data;
+  const CommandData *data = getItem();
 
   if (level == eDescriptionLevelBrief) {
     s->Printf(", commands = %s",
               (data && data->user_source.GetSize() > 0) ? "yes" : "no");
     return;
   }
 
   s->IndentMore();

lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp

 #include "lldb/Breakpoint/StoppointCallbackContext.h"
 #include "lldb/Core/IOHandler.h"
 #include "lldb/Core/State.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 
+#include "llvm/ADT/STLExtras.h"
+
 using namespace lldb;
 using namespace lldb_private;
 
 //-------------------------------------------------------------------------
 // CommandObjectBreakpointCommandAdd
 //-------------------------------------------------------------------------
 
 class CommandObjectBreakpointCommandAdd : public CommandObjectParsed,
void IOHandlerInputComplete(IOHandler &io_handler,
     io_handler.SetIsDone(true);
 
     std::vector<BreakpointOptions *> *bp_options_vec =
         (std::vector<BreakpointOptions *> *)io_handler.GetUserData();
     for (BreakpointOptions *bp_options : *bp_options_vec) {
       if (!bp_options)
         continue;
 
-      BreakpointOptions::CommandData *cmd_data =
-          new BreakpointOptions::CommandData();
+      auto cmd_data = llvm::make_unique<BreakpointOptions::CommandData>();
       cmd_data->user_source.SplitIntoLines(line.c_str(), line.size());
-      bp_options->SetCommandDataCallback(cmd_data);
+      bp_options->SetCommandDataCallback(std::move(cmd_data));
     }
   }
 
   void CollectDataForBreakpointCommandCallback(
       std::vector<BreakpointOptions *> &bp_options_vec,
       CommandReturnObject &result) {
     m_interpreter.GetLLDBCommandsFromIOHandler(
         "> ",             // Prompt
         *this,            // IOHandlerDelegate
         true,             // Run IOHandler in async mode
         &bp_options_vec); // Baton for the "io_handler" that will be passed back
                           // into our IOHandlerDelegate functions
   }
 
   /// Set a one-liner as the callback for the breakpoint.
   void
   SetBreakpointCommandCallback(std::vector<BreakpointOptions *> &bp_options_vec,
                                const char *oneliner) {
     for (auto bp_options : bp_options_vec) {
-      BreakpointOptions::CommandData *cmd_data =
-          new BreakpointOptions::CommandData();
+      auto cmd_data = llvm::make_unique<BreakpointOptions::CommandData>();
 
       // It's necessary to set both user_source and script_source to the
       // oneliner.
       // The former is used to generate callback description (as in breakpoint
       // command list)
       // while the latter is used for Python to interpret during the actual
       // callback.
       cmd_data->user_source.AppendString(oneliner);
       cmd_data->script_source.assign(oneliner);
       cmd_data->stop_on_error = m_options.m_stop_on_error;
 
-      bp_options->SetCommandDataCallback(cmd_data);
+      bp_options->SetCommandDataCallback(std::move(cmd_data));
     }
   }
 
   class CommandOptions : public Options {
   public:
     CommandOptions()
         : Options(), m_use_commands(false), m_use_script_language(false),
           m_script_language(eScriptLanguageNone), m_use_one_liner(false),

lldb/trunk/source/Commands/CommandObjectWatchpointCommand.cpp

void IOHandlerInputComplete(IOHandler &io_handler,
     // location
     WatchpointOptions *wp_options =
         (WatchpointOptions *)io_handler.GetUserData();
     if (wp_options) {
       std::unique_ptr<WatchpointOptions::CommandData> data_ap(
           new WatchpointOptions::CommandData());
       if (data_ap) {
         data_ap->user_source.SplitIntoLines(line);
-        BatonSP baton_sp(
-            new WatchpointOptions::CommandBaton(data_ap.release()));
+        auto baton_sp = std::make_shared<WatchpointOptions::CommandBaton>(
+            std::move(data_ap));
         wp_options->SetCallback(WatchpointOptionsCallbackFunction, baton_sp);
       }
     }
   }
 
   void CollectDataForWatchpointCommandCallback(WatchpointOptions *wp_options,
                                                CommandReturnObject &result) {
     m_interpreter.GetLLDBCommandsFromIOHandler(
void SetWatchpointCommandCallback(WatchpointOptions *wp_options,
     // The former is used to generate callback description (as in watchpoint
     // command list)
     // while the latter is used for Python to interpret during the actual
     // callback.
     data_ap->user_source.AppendString(oneliner);
     data_ap->script_source.assign(oneliner);
     data_ap->stop_on_error = m_options.m_stop_on_error;
 
-    BatonSP baton_sp(new WatchpointOptions::CommandBaton(data_ap.release()));
+    auto baton_sp =
+        std::make_shared<WatchpointOptions::CommandBaton>(std::move(data_ap));
     wp_options->SetCallback(WatchpointOptionsCallbackFunction, baton_sp);
   }
 
   static bool
   WatchpointOptionsCallbackFunction(void *baton,
                                     StoppointCallbackContext *context,
                                     lldb::user_id_t watch_id) {
     bool ret_value = true;

lldb/trunk/source/Core/Baton.cpp

 // C++ Includes
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Core/Stream.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
-void Baton::GetDescription(Stream *s, lldb::DescriptionLevel level) const {}
+void UntypedBaton::GetDescription(Stream *s,
+                                  lldb::DescriptionLevel level) const {}

lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

case eIOHandlerNone:
     break;
   case eIOHandlerBreakpoint: {
     std::vector<BreakpointOptions *> *bp_options_vec =
         (std::vector<BreakpointOptions *> *)io_handler.GetUserData();
     for (auto bp_options : *bp_options_vec) {
       if (!bp_options)
         continue;
 
-      std::unique_ptr<BreakpointOptions::CommandData> data_ap(
-          new BreakpointOptions::CommandData());
-      if (data_ap.get()) {
-        data_ap->user_source.SplitIntoLines(data);
+      auto data_ap = llvm::make_unique<BreakpointOptions::CommandData>();
+      if (!data_ap)
+        break;
 
-        if (GenerateBreakpointCommandCallbackData(data_ap->user_source,
-                                                  data_ap->script_source)
-                .Success()) {
-          BreakpointOptions::CommandBatonSP baton_sp(
-              new BreakpointOptions::CommandBaton(data_ap.release()));
-          bp_options->SetCallback(
-              ScriptInterpreterPython::BreakpointCallbackFunction, baton_sp);
-        } else if (!batch_mode) {
-          StreamFileSP error_sp = io_handler.GetErrorStreamFile();
-          if (error_sp) {
-            error_sp->Printf("Warning: No command attached to breakpoint.\n");
-            error_sp->Flush();
-          }
-        }
-      }
-    }
+        auto baton_sp = std::make_shared<BreakpointOptions::CommandBaton>(
+            std::move(data_ap));
     m_active_io_handler = eIOHandlerNone;
   } break;
   case eIOHandlerWatchpoint: {
     WatchpointOptions *wp_options =
         (WatchpointOptions *)io_handler.GetUserData();
-    std::unique_ptr<WatchpointOptions::CommandData> data_ap(
-        new WatchpointOptions::CommandData());
-    if (data_ap.get()) {
-      data_ap->user_source.SplitIntoLines(data);
+    auto data_ap = llvm::make_unique<WatchpointOptions::CommandData>();
 
-      if (GenerateWatchpointCommandCallbackData(data_ap->user_source,
-                                                data_ap->script_source)) {
-        BatonSP baton_sp(
-            new WatchpointOptions::CommandBaton(data_ap.release()));
-        wp_options->SetCallback(
-            ScriptInterpreterPython::WatchpointCallbackFunction, baton_sp);
-      } else if (!batch_mode) {
-        StreamFileSP error_sp = io_handler.GetErrorStreamFile();
-        if (error_sp) {
-          error_sp->Printf("Warning: No command attached to breakpoint.\n");
-          error_sp->Flush();
-        }
-      }
-    }
+      auto baton_sp =
+          std::make_shared<WatchpointOptions::CommandBaton>(std::move(data_ap));
     m_active_io_handler = eIOHandlerNone;
   } break;
   }
 }
 
 void ScriptInterpreterPython::ResetOutputFileHandle(FILE *fh) {}
 
 void ScriptInterpreterPython::SaveTerminalState(int fd) {
void ScriptInterpreterPython::SetBreakpointCommandCallbackFunction(
   oneliner += "(frame, bp_loc, internal_dict)";
   m_interpreter.GetScriptInterpreter()->SetBreakpointCommandCallback(
       bp_options, oneliner.c_str());
 }
 
 // Set a Python one-liner as the callback for the breakpoint.
 Error ScriptInterpreterPython::SetBreakpointCommandCallback(
     BreakpointOptions *bp_options, const char *command_body_text) {
-  std::unique_ptr<BreakpointOptions::CommandData> data_ap(
-      new BreakpointOptions::CommandData());
+  auto data_ap = llvm::make_unique<BreakpointOptions::CommandData>();
 
   // Split the command_body_text into lines, and pass that to
   // GenerateBreakpointCommandCallbackData.  That will
   // wrap the body in an auto-generated function, and return the function name
   // in script_source.  That is what
   // the callback will actually invoke.
 
   data_ap->user_source.SplitIntoLines(command_body_text);
   Error error = GenerateBreakpointCommandCallbackData(data_ap->user_source,
                                                       data_ap->script_source);
   if (error.Success()) {
-    BreakpointOptions::CommandBatonSP baton_sp(
-        new BreakpointOptions::CommandBaton(data_ap.release()));
+    auto baton_sp =
+        std::make_shared<BreakpointOptions::CommandBaton>(std::move(data_ap));
     bp_options->SetCallback(ScriptInterpreterPython::BreakpointCallbackFunction,
                             baton_sp);
     return error;
   } else
     return error;
 }
 
 // Set a Python one-liner as the callback for the watchpoint.
 void ScriptInterpreterPython::SetWatchpointCommandCallback(
     WatchpointOptions *wp_options, const char *oneliner) {
-  std::unique_ptr<WatchpointOptions::CommandData> data_ap(
-      new WatchpointOptions::CommandData());
+  auto data_ap = llvm::make_unique<WatchpointOptions::CommandData>();
 
   // It's necessary to set both user_source and script_source to the oneliner.
   // The former is used to generate callback description (as in watchpoint
   // command list)
   // while the latter is used for Python to interpret during the actual
   // callback.
 
   data_ap->user_source.AppendString(oneliner);
   data_ap->script_source.assign(oneliner);
 
   if (GenerateWatchpointCommandCallbackData(data_ap->user_source,
                                             data_ap->script_source)) {
-    BatonSP baton_sp(new WatchpointOptions::CommandBaton(data_ap.release()));
+    auto baton_sp =
+        std::make_shared<WatchpointOptions::CommandBaton>(std::move(data_ap));
     wp_options->SetCallback(ScriptInterpreterPython::WatchpointCallbackFunction,
                             baton_sp);
   }
 
   return;
 }
 
 Error ScriptInterpreterPython::ExportFunctionDefinitionToInterpreter(