================
@@ -53,44 +49,52 @@ bool WatchpointResource::Contains(addr_t addr) {
void WatchpointResource::AddOwner(const WatchpointSP &wp_sp) {
std::lock_guard<std::recursive_mutex> guard(m_owners_mutex);
- m_owners.Add(wp_sp);
+ m_owners.push_back(wp_sp);
}
void WatchpointResource::RemoveOwner(WatchpointSP &wp_sp) {
std::lock_guard<std::recursive_mutex> guard(m_owners_mutex);
- m_owners.Remove(wp_sp);
+ const auto &it = std::find(m_owners.begin(), m_owners.end(), wp_sp);
+ if (it != m_owners.end())
+ m_owners.erase(it);
}
size_t WatchpointResource::GetNumberOfOwners() {
std::lock_guard<std::recursive_mutex> guard(m_owners_mutex);
- return m_owners.GetSize();
+ return m_owners.size();
}
bool WatchpointResource::OwnersContains(WatchpointSP &wp_sp) {
std::lock_guard<std::recursive_mutex> guard(m_owners_mutex);
- return m_owners.Contains(wp_sp);
+ const auto &it = std::find(m_owners.begin(), m_owners.end(), wp_sp);
+ if (it != m_owners.end())
+ return true;
+ return false;
}
bool WatchpointResource::OwnersContains(const Watchpoint *wp) {
std::lock_guard<std::recursive_mutex> guard(m_owners_mutex);
- return m_owners.Contains(wp);
+ for (WatchpointCollection::const_iterator it = m_owners.begin();
+ it != m_owners.end(); ++it)
+ if ((*it).get() == wp)
+ return true;
+ return false;
}
WatchpointSP WatchpointResource::GetOwnerAtIndex(size_t idx) {
std::lock_guard<std::recursive_mutex> guard(m_owners_mutex);
- assert(idx < m_owners.GetSize());
- if (idx >= m_owners.GetSize())
+ lldbassert(idx < m_owners.size());
+ if (idx >= m_owners.size())
return {};
- return m_owners.GetByIndex(idx);
+ return m_owners[idx];
----------------
bulbazord wrote:
Well, I'm not entirely sure how we're going to want to access the Owners list,
so I'd like to understand more how you plan on using it. If you want to do
something like `wp_resource_sp->Owners()[2]` then this method makes sense to
keep around, but I'd also be curious to know why you would want to access the
3rd owner directly. What I would like to avoid is writing a loop like this:
```
for (int i = 0; i < wp_resource_sp.GetNumberOfOwners(); i++) {
WatchpointSP wp_sp = wp_resource_sp->GetOwnerAtIndex(i);
// Use wp_sp
}
```
The two main reasons I am not a fan of this pattern:
1. As you pointed out, one would need to grab the `wp_resource_sp` owner mutex
to do this safely since another thread may call `RemoveOwner` in the middle of
this loop. I can't say I'm a fan of exposing the vector primarily because it
would mean we'd have to expose the mutex too.
2. Off-by-one errors are fairly easy to put in by mistake. Doubly so if we ever
iterate in reverse (i >0 vs i >=0 for the termination condition).
If we don't need to be able to access any of the owners at an arbitrary index,
I would suggest removing it so nobody can write that kind of loop. WDYT?
https://github.com/llvm/llvm-project/pull/68845
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
