Skip to content

Commit 0cea54a

Browse files
authored
[lldb][NFCI] Remove EventData* param from BroadcastEvent (#78773)
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.
1 parent 04952c5 commit 0cea54a

File tree

12 files changed

+66
-68
lines changed

12 files changed

+66
-68
lines changed

lldb/include/lldb/Breakpoint/Watchpoint.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,6 @@ class Watchpoint : public std::enable_shared_from_this<Watchpoint>,
235235

236236
void SendWatchpointChangedEvent(lldb::WatchpointEventType eventKind);
237237

238-
void SendWatchpointChangedEvent(WatchpointEventData *data);
239-
240238
Watchpoint(const Watchpoint &) = delete;
241239
const Watchpoint &operator=(const Watchpoint &) = delete;
242240
};

lldb/include/lldb/Utility/Broadcaster.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,8 @@ class Broadcaster {
177177
m_broadcaster_sp->BroadcastEvent(event_type, event_data_sp);
178178
}
179179

180-
void BroadcastEvent(uint32_t event_type, EventData *event_data = nullptr) {
181-
m_broadcaster_sp->BroadcastEvent(event_type, event_data);
180+
void BroadcastEvent(uint32_t event_type) {
181+
m_broadcaster_sp->BroadcastEvent(event_type);
182182
}
183183

184184
void BroadcastEventIfUnique(uint32_t event_type,
@@ -346,7 +346,7 @@ class Broadcaster {
346346

347347
void BroadcastEventIfUnique(lldb::EventSP &event_sp);
348348

349-
void BroadcastEvent(uint32_t event_type, EventData *event_data = nullptr);
349+
void BroadcastEvent(uint32_t event_type);
350350

351351
void BroadcastEvent(uint32_t event_type,
352352
const lldb::EventDataSP &event_data_sp);

lldb/source/Breakpoint/BreakpointList.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,12 @@ using namespace lldb_private;
1717

1818
static void NotifyChange(const BreakpointSP &bp, BreakpointEventType event) {
1919
Target &target = bp->GetTarget();
20-
if (target.EventTypeHasListeners(Target::eBroadcastBitBreakpointChanged))
20+
if (target.EventTypeHasListeners(Target::eBroadcastBitBreakpointChanged)) {
21+
auto event_data_sp =
22+
std::make_shared<Breakpoint::BreakpointEventData>(event, bp);
2123
target.BroadcastEvent(Target::eBroadcastBitBreakpointChanged,
22-
new Breakpoint::BreakpointEventData(event, bp));
24+
event_data_sp);
25+
}
2326
}
2427

2528
BreakpointList::BreakpointList(bool is_internal)

lldb/source/Breakpoint/BreakpointLocation.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -649,11 +649,11 @@ void BreakpointLocation::SendBreakpointLocationChangedEvent(
649649
if (!m_being_created && !m_owner.IsInternal() &&
650650
m_owner.GetTarget().EventTypeHasListeners(
651651
Target::eBroadcastBitBreakpointChanged)) {
652-
Breakpoint::BreakpointEventData *data = new Breakpoint::BreakpointEventData(
652+
auto data_sp = std::make_shared<Breakpoint::BreakpointEventData>(
653653
eventKind, m_owner.shared_from_this());
654-
data->GetBreakpointLocationCollection().Add(shared_from_this());
654+
data_sp->GetBreakpointLocationCollection().Add(shared_from_this());
655655
m_owner.GetTarget().BroadcastEvent(Target::eBroadcastBitBreakpointChanged,
656-
data);
656+
data_sp);
657657
}
658658
}
659659

lldb/source/Breakpoint/Watchpoint.cpp

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -462,26 +462,14 @@ const char *Watchpoint::GetConditionText() const {
462462

463463
void Watchpoint::SendWatchpointChangedEvent(
464464
lldb::WatchpointEventType eventKind) {
465-
if (!m_being_created &&
466-
GetTarget().EventTypeHasListeners(
467-
Target::eBroadcastBitWatchpointChanged)) {
468-
WatchpointEventData *data =
469-
new Watchpoint::WatchpointEventData(eventKind, shared_from_this());
470-
GetTarget().BroadcastEvent(Target::eBroadcastBitWatchpointChanged, data);
465+
if (!m_being_created && GetTarget().EventTypeHasListeners(
466+
Target::eBroadcastBitWatchpointChanged)) {
467+
auto data_sp =
468+
std::make_shared<WatchpointEventData>(eventKind, shared_from_this());
469+
GetTarget().BroadcastEvent(Target::eBroadcastBitWatchpointChanged, data_sp);
471470
}
472471
}
473472

474-
void Watchpoint::SendWatchpointChangedEvent(WatchpointEventData *data) {
475-
if (data == nullptr)
476-
return;
477-
478-
if (!m_being_created &&
479-
GetTarget().EventTypeHasListeners(Target::eBroadcastBitWatchpointChanged))
480-
GetTarget().BroadcastEvent(Target::eBroadcastBitWatchpointChanged, data);
481-
else
482-
delete data;
483-
}
484-
485473
Watchpoint::WatchpointEventData::WatchpointEventData(
486474
WatchpointEventType sub_type, const WatchpointSP &new_watchpoint_sp)
487475
: m_watchpoint_event(sub_type), m_new_watchpoint_sp(new_watchpoint_sp) {}

lldb/source/Breakpoint/WatchpointList.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,12 @@ lldb::watch_id_t WatchpointList::Add(const WatchpointSP &wp_sp, bool notify) {
2323
m_watchpoints.push_back(wp_sp);
2424
if (notify) {
2525
if (wp_sp->GetTarget().EventTypeHasListeners(
26-
Target::eBroadcastBitWatchpointChanged))
26+
Target::eBroadcastBitWatchpointChanged)) {
27+
auto data_sp = std::make_shared<Watchpoint::WatchpointEventData>(
28+
eWatchpointEventTypeAdded, wp_sp);
2729
wp_sp->GetTarget().BroadcastEvent(Target::eBroadcastBitWatchpointChanged,
28-
new Watchpoint::WatchpointEventData(
29-
eWatchpointEventTypeAdded, wp_sp));
30+
data_sp);
31+
}
3032
}
3133
return wp_sp->GetID();
3234
}
@@ -171,11 +173,12 @@ bool WatchpointList::Remove(lldb::watch_id_t watch_id, bool notify) {
171173
WatchpointSP wp_sp = *pos;
172174
if (notify) {
173175
if (wp_sp->GetTarget().EventTypeHasListeners(
174-
Target::eBroadcastBitWatchpointChanged))
176+
Target::eBroadcastBitWatchpointChanged)) {
177+
auto data_sp = std::make_shared<Watchpoint::WatchpointEventData>(
178+
eWatchpointEventTypeRemoved, wp_sp);
175179
wp_sp->GetTarget().BroadcastEvent(
176-
Target::eBroadcastBitWatchpointChanged,
177-
new Watchpoint::WatchpointEventData(eWatchpointEventTypeRemoved,
178-
wp_sp));
180+
Target::eBroadcastBitWatchpointChanged, data_sp);
181+
}
179182
}
180183
m_watchpoints.erase(pos);
181184
return true;
@@ -234,10 +237,10 @@ void WatchpointList::RemoveAll(bool notify) {
234237
for (pos = m_watchpoints.begin(); pos != end; ++pos) {
235238
if ((*pos)->GetTarget().EventTypeHasListeners(
236239
Target::eBroadcastBitBreakpointChanged)) {
240+
auto data_sp = std::make_shared<Watchpoint::WatchpointEventData>(
241+
eWatchpointEventTypeRemoved, *pos);
237242
(*pos)->GetTarget().BroadcastEvent(
238-
Target::eBroadcastBitWatchpointChanged,
239-
new Watchpoint::WatchpointEventData(eWatchpointEventTypeRemoved,
240-
*pos));
243+
Target::eBroadcastBitWatchpointChanged, data_sp);
241244
}
242245
}
243246
}

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,8 +1089,8 @@ Status ProcessGDBRemote::DoAttachToProcessWithID(
10891089
const int packet_len =
10901090
::snprintf(packet, sizeof(packet), "vAttach;%" PRIx64, attach_pid);
10911091
SetID(attach_pid);
1092-
m_async_broadcaster.BroadcastEvent(
1093-
eBroadcastBitAsyncContinue, new EventDataBytes(packet, packet_len));
1092+
auto data_sp = std::make_shared<EventDataBytes>(packet, packet_len);
1093+
m_async_broadcaster.BroadcastEvent(eBroadcastBitAsyncContinue, data_sp);
10941094
} else
10951095
SetExitStatus(-1, error.AsCString());
10961096
}
@@ -1127,9 +1127,9 @@ Status ProcessGDBRemote::DoAttachToProcessWithName(
11271127
endian::InlHostByteOrder(),
11281128
endian::InlHostByteOrder());
11291129
1130-
m_async_broadcaster.BroadcastEvent(
1131-
eBroadcastBitAsyncContinue,
1132-
new EventDataBytes(packet.GetString().data(), packet.GetSize()));
1130+
auto data_sp = std::make_shared<EventDataBytes>(packet.GetString().data(),
1131+
packet.GetSize());
1132+
m_async_broadcaster.BroadcastEvent(eBroadcastBitAsyncContinue, data_sp);
11331133
11341134
} else
11351135
SetExitStatus(-1, error.AsCString());
@@ -1374,10 +1374,9 @@ Status ProcessGDBRemote::DoResume() {
13741374
return error;
13751375
}
13761376
1377-
m_async_broadcaster.BroadcastEvent(
1378-
eBroadcastBitAsyncContinue,
1379-
new EventDataBytes(continue_packet.GetString().data(),
1380-
continue_packet.GetSize()));
1377+
auto data_sp = std::make_shared<EventDataBytes>(
1378+
continue_packet.GetString().data(), continue_packet.GetSize());
1379+
m_async_broadcaster.BroadcastEvent(eBroadcastBitAsyncContinue, data_sp);
13811380
13821381
if (!listener_sp->GetEvent(event_sp, std::chrono::seconds(5))) {
13831382
error.SetErrorString("Resume timed out.");

lldb/source/Target/Process.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4304,9 +4304,9 @@ void Process::BroadcastAsyncProfileData(const std::string &one_profile_data) {
43044304

43054305
void Process::BroadcastStructuredData(const StructuredData::ObjectSP &object_sp,
43064306
const StructuredDataPluginSP &plugin_sp) {
4307-
BroadcastEvent(
4308-
eBroadcastBitStructuredData,
4309-
new EventDataStructuredData(shared_from_this(), object_sp, plugin_sp));
4307+
auto data_sp = std::make_shared<EventDataStructuredData>(
4308+
shared_from_this(), object_sp, plugin_sp);
4309+
BroadcastEvent(eBroadcastBitStructuredData, data_sp);
43104310
}
43114311

43124312
StructuredDataPluginSP

lldb/source/Target/Target.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1704,8 +1704,9 @@ void Target::ModulesDidLoad(ModuleList &module_list) {
17041704
if (m_process_sp) {
17051705
m_process_sp->ModulesDidLoad(module_list);
17061706
}
1707-
BroadcastEvent(eBroadcastBitModulesLoaded,
1708-
new TargetEventData(this->shared_from_this(), module_list));
1707+
auto data_sp =
1708+
std::make_shared<TargetEventData>(shared_from_this(), module_list);
1709+
BroadcastEvent(eBroadcastBitModulesLoaded, data_sp);
17091710
}
17101711
}
17111712

@@ -1719,16 +1720,18 @@ void Target::SymbolsDidLoad(ModuleList &module_list) {
17191720

17201721
m_breakpoint_list.UpdateBreakpoints(module_list, true, false);
17211722
m_internal_breakpoint_list.UpdateBreakpoints(module_list, true, false);
1722-
BroadcastEvent(eBroadcastBitSymbolsLoaded,
1723-
new TargetEventData(this->shared_from_this(), module_list));
1723+
auto data_sp =
1724+
std::make_shared<TargetEventData>(shared_from_this(), module_list);
1725+
BroadcastEvent(eBroadcastBitSymbolsLoaded, data_sp);
17241726
}
17251727
}
17261728

17271729
void Target::ModulesDidUnload(ModuleList &module_list, bool delete_locations) {
17281730
if (m_valid && module_list.GetSize()) {
17291731
UnloadModuleSections(module_list);
1730-
BroadcastEvent(eBroadcastBitModulesUnloaded,
1731-
new TargetEventData(this->shared_from_this(), module_list));
1732+
auto data_sp =
1733+
std::make_shared<TargetEventData>(shared_from_this(), module_list);
1734+
BroadcastEvent(eBroadcastBitModulesUnloaded, data_sp);
17321735
m_breakpoint_list.UpdateBreakpoints(module_list, false, delete_locations);
17331736
m_internal_breakpoint_list.UpdateBreakpoints(module_list, false,
17341737
delete_locations);

lldb/source/Target/Thread.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -253,9 +253,11 @@ void Thread::DestroyThread() {
253253
}
254254

255255
void Thread::BroadcastSelectedFrameChange(StackID &new_frame_id) {
256-
if (EventTypeHasListeners(eBroadcastBitSelectedFrameChanged))
257-
BroadcastEvent(eBroadcastBitSelectedFrameChanged,
258-
new ThreadEventData(this->shared_from_this(), new_frame_id));
256+
if (EventTypeHasListeners(eBroadcastBitSelectedFrameChanged)) {
257+
auto data_sp =
258+
std::make_shared<ThreadEventData>(shared_from_this(), new_frame_id);
259+
BroadcastEvent(eBroadcastBitSelectedFrameChanged, data_sp);
260+
}
259261
}
260262

261263
lldb::StackFrameSP
@@ -1507,9 +1509,10 @@ Status Thread::ReturnFromFrame(lldb::StackFrameSP frame_sp,
15071509
if (copy_success) {
15081510
thread->DiscardThreadPlans(true);
15091511
thread->ClearStackFrames();
1510-
if (broadcast && EventTypeHasListeners(eBroadcastBitStackChanged))
1511-
BroadcastEvent(eBroadcastBitStackChanged,
1512-
new ThreadEventData(this->shared_from_this()));
1512+
if (broadcast && EventTypeHasListeners(eBroadcastBitStackChanged)) {
1513+
auto data_sp = std::make_shared<ThreadEventData>(shared_from_this());
1514+
BroadcastEvent(eBroadcastBitStackChanged, data_sp);
1515+
}
15131516
} else {
15141517
return_error.SetErrorString("Could not reset register values.");
15151518
}

lldb/source/Target/ThreadList.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -726,10 +726,12 @@ bool ThreadList::SetSelectedThreadByIndexID(uint32_t index_id, bool notify) {
726726
void ThreadList::NotifySelectedThreadChanged(lldb::tid_t tid) {
727727
ThreadSP selected_thread_sp(FindThreadByID(tid));
728728
if (selected_thread_sp->EventTypeHasListeners(
729-
Thread::eBroadcastBitThreadSelected))
730-
selected_thread_sp->BroadcastEvent(
731-
Thread::eBroadcastBitThreadSelected,
732-
new Thread::ThreadEventData(selected_thread_sp));
729+
Thread::eBroadcastBitThreadSelected)) {
730+
auto data_sp =
731+
std::make_shared<Thread::ThreadEventData>(selected_thread_sp);
732+
selected_thread_sp->BroadcastEvent(Thread::eBroadcastBitThreadSelected,
733+
data_sp);
734+
}
733735
}
734736

735737
void ThreadList::Update(ThreadList &rhs) {

lldb/source/Utility/Broadcaster.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -300,9 +300,8 @@ void Broadcaster::BroadcasterImpl::PrivateBroadcastEvent(EventSP &event_sp,
300300
}
301301
}
302302

303-
void Broadcaster::BroadcasterImpl::BroadcastEvent(uint32_t event_type,
304-
EventData *event_data) {
305-
auto event_sp = std::make_shared<Event>(event_type, event_data);
303+
void Broadcaster::BroadcasterImpl::BroadcastEvent(uint32_t event_type) {
304+
auto event_sp = std::make_shared<Event>(event_type, /*data = */ nullptr);
306305
PrivateBroadcastEvent(event_sp, false);
307306
}
308307

0 commit comments

Comments
 (0)