[Lldb-commits] [lldb] [lldb][NFCI] Remove EventData* param from BroadcastEvent (PR #78773)
via lldb-commits
lldb-commits at lists.llvm.org
Fri Jan 19 12:07:22 PST 2024
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
More information about the lldb-commits
mailing list