[lldb][NFCI] Remove EventData* param from BroadcastEvent #78773
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
|
@llvm/pr-subscribers-lldb Author: Alex Langford (bulbazord) ChangesBroadcastEvent 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 Full diff: https://github.com/llvm/llvm-project/pull/78773.diff 12 Files Affected:
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);
}
|
JDevlieghere
approved these changes
Jan 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
newand passing raw pointers around, I think it would make more sense to create the shared_ptr up front.