llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Alex Langford (bulbazord) <details> <summary>Changes</summary> BroadcastEvent currently takes its EventData* param and shoves it into an Event object, which takes ownership of the pointer and places it into a shared_ptr to manage the lifetime. Instead of relying on `new` and passing raw pointers around, I think it would make more sense to create the shared_ptr up front. --- Full diff: https://github.com/llvm/llvm-project/pull/78773.diff 12 Files Affected: - (modified) lldb/include/lldb/Breakpoint/Watchpoint.h (-2) - (modified) lldb/include/lldb/Utility/Broadcaster.h (+3-3) - (modified) lldb/source/Breakpoint/BreakpointList.cpp (+5-2) - (modified) lldb/source/Breakpoint/BreakpointLocation.cpp (+3-3) - (modified) lldb/source/Breakpoint/Watchpoint.cpp (+5-17) - (modified) lldb/source/Breakpoint/WatchpointList.cpp (+13-10) - (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+8-9) - (modified) lldb/source/Target/Process.cpp (+3-3) - (modified) lldb/source/Target/Target.cpp (+9-6) - (modified) lldb/source/Target/Thread.cpp (+9-6) - (modified) lldb/source/Target/ThreadList.cpp (+6-4) - (modified) lldb/source/Utility/Broadcaster.cpp (+2-3) ``````````diff diff --git a/lldb/include/lldb/Breakpoint/Watchpoint.h b/lldb/include/lldb/Breakpoint/Watchpoint.h index 851162af24c74e0..22fdfd686c3f115 100644 --- a/lldb/include/lldb/Breakpoint/Watchpoint.h +++ b/lldb/include/lldb/Breakpoint/Watchpoint.h @@ -235,8 +235,6 @@ class Watchpoint : public std::enable_shared_from_this<Watchpoint>, void SendWatchpointChangedEvent(lldb::WatchpointEventType eventKind); - void SendWatchpointChangedEvent(WatchpointEventData *data); - Watchpoint(const Watchpoint &) = delete; const Watchpoint &operator=(const Watchpoint &) = delete; }; diff --git a/lldb/include/lldb/Utility/Broadcaster.h b/lldb/include/lldb/Utility/Broadcaster.h index 8444c38f6ecc650..c8127f0a921d882 100644 --- a/lldb/include/lldb/Utility/Broadcaster.h +++ b/lldb/include/lldb/Utility/Broadcaster.h @@ -177,8 +177,8 @@ class Broadcaster { m_broadcaster_sp->BroadcastEvent(event_type, event_data_sp); } - void BroadcastEvent(uint32_t event_type, EventData *event_data = nullptr) { - m_broadcaster_sp->BroadcastEvent(event_type, event_data); + void BroadcastEvent(uint32_t event_type) { + m_broadcaster_sp->BroadcastEvent(event_type); } void BroadcastEventIfUnique(uint32_t event_type, @@ -346,7 +346,7 @@ class Broadcaster { void BroadcastEventIfUnique(lldb::EventSP &event_sp); - void BroadcastEvent(uint32_t event_type, EventData *event_data = nullptr); + void BroadcastEvent(uint32_t event_type); void BroadcastEvent(uint32_t event_type, const lldb::EventDataSP &event_data_sp); diff --git a/lldb/source/Breakpoint/BreakpointList.cpp b/lldb/source/Breakpoint/BreakpointList.cpp index f7c2cdb938e62af..2c47b3b1263c65b 100644 --- a/lldb/source/Breakpoint/BreakpointList.cpp +++ b/lldb/source/Breakpoint/BreakpointList.cpp @@ -17,9 +17,12 @@ using namespace lldb_private; static void NotifyChange(const BreakpointSP &bp, BreakpointEventType event) { Target &target = bp->GetTarget(); - if (target.EventTypeHasListeners(Target::eBroadcastBitBreakpointChanged)) + if (target.EventTypeHasListeners(Target::eBroadcastBitBreakpointChanged)) { + auto event_data_sp = + std::make_shared<Breakpoint::BreakpointEventData>(event, bp); target.BroadcastEvent(Target::eBroadcastBitBreakpointChanged, - new Breakpoint::BreakpointEventData(event, bp)); + event_data_sp); + } } BreakpointList::BreakpointList(bool is_internal) diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp index 99f94d04bb31843..98de059c2e29677 100644 --- a/lldb/source/Breakpoint/BreakpointLocation.cpp +++ b/lldb/source/Breakpoint/BreakpointLocation.cpp @@ -649,11 +649,11 @@ void BreakpointLocation::SendBreakpointLocationChangedEvent( if (!m_being_created && !m_owner.IsInternal() && m_owner.GetTarget().EventTypeHasListeners( Target::eBroadcastBitBreakpointChanged)) { - Breakpoint::BreakpointEventData *data = new Breakpoint::BreakpointEventData( + auto data_sp = std::make_shared<Breakpoint::BreakpointEventData>( eventKind, m_owner.shared_from_this()); - data->GetBreakpointLocationCollection().Add(shared_from_this()); + data_sp->GetBreakpointLocationCollection().Add(shared_from_this()); m_owner.GetTarget().BroadcastEvent(Target::eBroadcastBitBreakpointChanged, - data); + data_sp); } } diff --git a/lldb/source/Breakpoint/Watchpoint.cpp b/lldb/source/Breakpoint/Watchpoint.cpp index 4602ce4213b9cda..1a8ef87c1e67a32 100644 --- a/lldb/source/Breakpoint/Watchpoint.cpp +++ b/lldb/source/Breakpoint/Watchpoint.cpp @@ -462,26 +462,14 @@ const char *Watchpoint::GetConditionText() const { void Watchpoint::SendWatchpointChangedEvent( lldb::WatchpointEventType eventKind) { - if (!m_being_created && - GetTarget().EventTypeHasListeners( - Target::eBroadcastBitWatchpointChanged)) { - WatchpointEventData *data = - new Watchpoint::WatchpointEventData(eventKind, shared_from_this()); - GetTarget().BroadcastEvent(Target::eBroadcastBitWatchpointChanged, data); + if (!m_being_created && GetTarget().EventTypeHasListeners( + Target::eBroadcastBitWatchpointChanged)) { + auto data_sp = + std::make_shared<WatchpointEventData>(eventKind, shared_from_this()); + GetTarget().BroadcastEvent(Target::eBroadcastBitWatchpointChanged, data_sp); } } -void Watchpoint::SendWatchpointChangedEvent(WatchpointEventData *data) { - if (data == nullptr) - return; - - if (!m_being_created && - GetTarget().EventTypeHasListeners(Target::eBroadcastBitWatchpointChanged)) - GetTarget().BroadcastEvent(Target::eBroadcastBitWatchpointChanged, data); - else - delete data; -} - Watchpoint::WatchpointEventData::WatchpointEventData( WatchpointEventType sub_type, const WatchpointSP &new_watchpoint_sp) : m_watchpoint_event(sub_type), m_new_watchpoint_sp(new_watchpoint_sp) {} diff --git a/lldb/source/Breakpoint/WatchpointList.cpp b/lldb/source/Breakpoint/WatchpointList.cpp index fc0cc9126d602e1..f7564483e6f1fcd 100644 --- a/lldb/source/Breakpoint/WatchpointList.cpp +++ b/lldb/source/Breakpoint/WatchpointList.cpp @@ -23,10 +23,12 @@ lldb::watch_id_t WatchpointList::Add(const WatchpointSP &wp_sp, bool notify) { m_watchpoints.push_back(wp_sp); if (notify) { if (wp_sp->GetTarget().EventTypeHasListeners( - Target::eBroadcastBitWatchpointChanged)) + Target::eBroadcastBitWatchpointChanged)) { + auto data_sp = std::make_shared<Watchpoint::WatchpointEventData>( + eWatchpointEventTypeAdded, wp_sp); wp_sp->GetTarget().BroadcastEvent(Target::eBroadcastBitWatchpointChanged, - new Watchpoint::WatchpointEventData( - eWatchpointEventTypeAdded, wp_sp)); + data_sp); + } } return wp_sp->GetID(); } @@ -171,11 +173,12 @@ bool WatchpointList::Remove(lldb::watch_id_t watch_id, bool notify) { WatchpointSP wp_sp = *pos; if (notify) { if (wp_sp->GetTarget().EventTypeHasListeners( - Target::eBroadcastBitWatchpointChanged)) + Target::eBroadcastBitWatchpointChanged)) { + auto data_sp = std::make_shared<Watchpoint::WatchpointEventData>( + eWatchpointEventTypeRemoved, wp_sp); wp_sp->GetTarget().BroadcastEvent( - Target::eBroadcastBitWatchpointChanged, - new Watchpoint::WatchpointEventData(eWatchpointEventTypeRemoved, - wp_sp)); + Target::eBroadcastBitWatchpointChanged, data_sp); + } } m_watchpoints.erase(pos); return true; @@ -234,10 +237,10 @@ void WatchpointList::RemoveAll(bool notify) { for (pos = m_watchpoints.begin(); pos != end; ++pos) { if ((*pos)->GetTarget().EventTypeHasListeners( Target::eBroadcastBitBreakpointChanged)) { + auto data_sp = std::make_shared<Watchpoint::WatchpointEventData>( + eWatchpointEventTypeRemoved, *pos); (*pos)->GetTarget().BroadcastEvent( - Target::eBroadcastBitWatchpointChanged, - new Watchpoint::WatchpointEventData(eWatchpointEventTypeRemoved, - *pos)); + Target::eBroadcastBitWatchpointChanged, data_sp); } } } diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 23834d403579c64..eb42b9eb6cb6a57 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1089,8 +1089,8 @@ Status ProcessGDBRemote::DoAttachToProcessWithID( const int packet_len = ::snprintf(packet, sizeof(packet), "vAttach;%" PRIx64, attach_pid); SetID(attach_pid); - m_async_broadcaster.BroadcastEvent( - eBroadcastBitAsyncContinue, new EventDataBytes(packet, packet_len)); + auto data_sp = std::make_shared<EventDataBytes>(packet, packet_len); + m_async_broadcaster.BroadcastEvent(eBroadcastBitAsyncContinue, data_sp); } else SetExitStatus(-1, error.AsCString()); } @@ -1127,9 +1127,9 @@ Status ProcessGDBRemote::DoAttachToProcessWithName( endian::InlHostByteOrder(), endian::InlHostByteOrder()); - m_async_broadcaster.BroadcastEvent( - eBroadcastBitAsyncContinue, - new EventDataBytes(packet.GetString().data(), packet.GetSize())); + auto data_sp = std::make_shared<EventDataBytes>(packet.GetString().data(), + packet.GetSize()); + m_async_broadcaster.BroadcastEvent(eBroadcastBitAsyncContinue, data_sp); } else SetExitStatus(-1, error.AsCString()); @@ -1374,10 +1374,9 @@ Status ProcessGDBRemote::DoResume() { return error; } - m_async_broadcaster.BroadcastEvent( - eBroadcastBitAsyncContinue, - new EventDataBytes(continue_packet.GetString().data(), - continue_packet.GetSize())); + auto data_sp = std::make_shared<EventDataBytes>( + continue_packet.GetString().data(), continue_packet.GetSize()); + m_async_broadcaster.BroadcastEvent(eBroadcastBitAsyncContinue, data_sp); if (!listener_sp->GetEvent(event_sp, std::chrono::seconds(5))) { error.SetErrorString("Resume timed out."); diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 8158bf61258378c..e1c16ca21643227 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -4304,9 +4304,9 @@ void Process::BroadcastAsyncProfileData(const std::string &one_profile_data) { void Process::BroadcastStructuredData(const StructuredData::ObjectSP &object_sp, const StructuredDataPluginSP &plugin_sp) { - BroadcastEvent( - eBroadcastBitStructuredData, - new EventDataStructuredData(shared_from_this(), object_sp, plugin_sp)); + auto data_sp = std::make_shared<EventDataStructuredData>( + shared_from_this(), object_sp, plugin_sp); + BroadcastEvent(eBroadcastBitStructuredData, data_sp); } StructuredDataPluginSP diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 302c2bad7021b9f..e969340fdf1eba5 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -1704,8 +1704,9 @@ void Target::ModulesDidLoad(ModuleList &module_list) { if (m_process_sp) { m_process_sp->ModulesDidLoad(module_list); } - BroadcastEvent(eBroadcastBitModulesLoaded, - new TargetEventData(this->shared_from_this(), module_list)); + auto data_sp = + std::make_shared<TargetEventData>(shared_from_this(), module_list); + BroadcastEvent(eBroadcastBitModulesLoaded, data_sp); } } @@ -1719,16 +1720,18 @@ void Target::SymbolsDidLoad(ModuleList &module_list) { m_breakpoint_list.UpdateBreakpoints(module_list, true, false); m_internal_breakpoint_list.UpdateBreakpoints(module_list, true, false); - BroadcastEvent(eBroadcastBitSymbolsLoaded, - new TargetEventData(this->shared_from_this(), module_list)); + auto data_sp = + std::make_shared<TargetEventData>(shared_from_this(), module_list); + BroadcastEvent(eBroadcastBitSymbolsLoaded, data_sp); } } void Target::ModulesDidUnload(ModuleList &module_list, bool delete_locations) { if (m_valid && module_list.GetSize()) { UnloadModuleSections(module_list); - BroadcastEvent(eBroadcastBitModulesUnloaded, - new TargetEventData(this->shared_from_this(), module_list)); + auto data_sp = + std::make_shared<TargetEventData>(shared_from_this(), module_list); + BroadcastEvent(eBroadcastBitModulesUnloaded, data_sp); m_breakpoint_list.UpdateBreakpoints(module_list, false, delete_locations); m_internal_breakpoint_list.UpdateBreakpoints(module_list, false, delete_locations); diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp index 865cee97e6d8783..8ae2179c1281d04 100644 --- a/lldb/source/Target/Thread.cpp +++ b/lldb/source/Target/Thread.cpp @@ -253,9 +253,11 @@ void Thread::DestroyThread() { } void Thread::BroadcastSelectedFrameChange(StackID &new_frame_id) { - if (EventTypeHasListeners(eBroadcastBitSelectedFrameChanged)) - BroadcastEvent(eBroadcastBitSelectedFrameChanged, - new ThreadEventData(this->shared_from_this(), new_frame_id)); + if (EventTypeHasListeners(eBroadcastBitSelectedFrameChanged)) { + auto data_sp = + std::make_shared<ThreadEventData>(shared_from_this(), new_frame_id); + BroadcastEvent(eBroadcastBitSelectedFrameChanged, data_sp); + } } lldb::StackFrameSP @@ -1507,9 +1509,10 @@ Status Thread::ReturnFromFrame(lldb::StackFrameSP frame_sp, if (copy_success) { thread->DiscardThreadPlans(true); thread->ClearStackFrames(); - if (broadcast && EventTypeHasListeners(eBroadcastBitStackChanged)) - BroadcastEvent(eBroadcastBitStackChanged, - new ThreadEventData(this->shared_from_this())); + if (broadcast && EventTypeHasListeners(eBroadcastBitStackChanged)) { + auto data_sp = std::make_shared<ThreadEventData>(shared_from_this()); + BroadcastEvent(eBroadcastBitStackChanged, data_sp); + } } else { return_error.SetErrorString("Could not reset register values."); } diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp index 1ba0c435b993d3e..03e8daedff12936 100644 --- a/lldb/source/Target/ThreadList.cpp +++ b/lldb/source/Target/ThreadList.cpp @@ -726,10 +726,12 @@ bool ThreadList::SetSelectedThreadByIndexID(uint32_t index_id, bool notify) { void ThreadList::NotifySelectedThreadChanged(lldb::tid_t tid) { ThreadSP selected_thread_sp(FindThreadByID(tid)); if (selected_thread_sp->EventTypeHasListeners( - Thread::eBroadcastBitThreadSelected)) - selected_thread_sp->BroadcastEvent( - Thread::eBroadcastBitThreadSelected, - new Thread::ThreadEventData(selected_thread_sp)); + Thread::eBroadcastBitThreadSelected)) { + auto data_sp = + std::make_shared<Thread::ThreadEventData>(selected_thread_sp); + selected_thread_sp->BroadcastEvent(Thread::eBroadcastBitThreadSelected, + data_sp); + } } void ThreadList::Update(ThreadList &rhs) { diff --git a/lldb/source/Utility/Broadcaster.cpp b/lldb/source/Utility/Broadcaster.cpp index 914812d7857779f..33cd49963e7c74c 100644 --- a/lldb/source/Utility/Broadcaster.cpp +++ b/lldb/source/Utility/Broadcaster.cpp @@ -300,9 +300,8 @@ void Broadcaster::BroadcasterImpl::PrivateBroadcastEvent(EventSP &event_sp, } } -void Broadcaster::BroadcasterImpl::BroadcastEvent(uint32_t event_type, - EventData *event_data) { - auto event_sp = std::make_shared<Event>(event_type, event_data); +void Broadcaster::BroadcasterImpl::BroadcastEvent(uint32_t event_type) { + auto event_sp = std::make_shared<Event>(event_type, /*data = */ nullptr); PrivateBroadcastEvent(event_sp, false); } `````````` </details> https://github.com/llvm/llvm-project/pull/78773 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
