Author: jdevlieghere
Date: Thu May 23 21:41:47 2019
New Revision: 361597
URL: http://llvm.org/viewvc/llvm-project?rev=361597&view=rev
Log:
[Utility] Small improvements to the Broadcaster class (NFC)
I touched the Broadcaster class earlier today (r361544) and noticed a
few things that could be improved. This patch includes variety of small
fixes: use early returns, use LLDB_LOG macro, use doxygen comments and
finally format the class.
Modified:
lldb/trunk/include/lldb/Utility/Broadcaster.h
lldb/trunk/source/Utility/Broadcaster.cpp
Modified: lldb/trunk/include/lldb/Utility/Broadcaster.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/Broadcaster.h?rev=361597&r1=361596&r2=361597&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Utility/Broadcaster.h (original)
+++ lldb/trunk/include/lldb/Utility/Broadcaster.h Thu May 23 21:41:47 2019
@@ -29,14 +29,14 @@ class Broadcaster;
class EventData;
class Listener;
class Stream;
-}
+} // namespace lldb_private
namespace lldb_private {
-// lldb::BroadcastEventSpec
-//
-// This class is used to specify a kind of event to register for. The Debugger
-// maintains a list of BroadcastEventSpec's and when it is made
+/// lldb::BroadcastEventSpec
+///
+/// This class is used to specify a kind of event to register for. The
+/// Debugger maintains a list of BroadcastEventSpec's and when it is made
class BroadcastEventSpec {
public:
BroadcastEventSpec(ConstString broadcaster_class, uint32_t event_bits)
@@ -48,19 +48,19 @@ public:
uint32_t GetEventBits() const { return m_event_bits; }
- // Tell whether this BroadcastEventSpec is contained in in_spec. That is: (a)
- // the two spec's share the same broadcaster class (b) the event bits of this
- // spec are wholly contained in those of in_spec.
+ /// Tell whether this BroadcastEventSpec is contained in in_spec. That is:
+ /// (a) the two spec's share the same broadcaster class (b) the event bits of
+ /// this spec are wholly contained in those of in_spec.
bool IsContainedIn(const BroadcastEventSpec &in_spec) const {
if (m_broadcaster_class != in_spec.GetBroadcasterClass())
return false;
uint32_t in_bits = in_spec.GetEventBits();
if (in_bits == m_event_bits)
return true;
- else {
- if ((m_event_bits & in_bits) != 0 && (m_event_bits & ~in_bits) == 0)
- return true;
- }
+
+ if ((m_event_bits & in_bits) != 0 && (m_event_bits & ~in_bits) == 0)
+ return true;
+
return false;
}
@@ -81,10 +81,9 @@ protected:
BroadcasterManager();
public:
- // Listeners hold onto weak pointers to their broadcaster managers. So they
- // must be made into shared pointers, which you do with
- // MakeBroadcasterManager.
-
+ /// Listeners hold onto weak pointers to their broadcaster managers. So they
+ /// must be made into shared pointers, which you do with
+ /// MakeBroadcasterManager.
static lldb::BroadcasterManagerSP MakeBroadcasterManager();
~BroadcasterManager() = default;
@@ -179,8 +178,8 @@ private:
bool operator()(const event_listener_key &input) const {
if (input.second == m_listener_sp)
return true;
- else
- return false;
+
+ return false;
}
private:
@@ -197,15 +196,15 @@ private:
bool operator()(const event_listener_key &input) const {
if (input.second.get() == m_listener)
return true;
- else
- return false;
+
+ return false;
}
bool operator()(const lldb::ListenerSP &input) const {
if (input.get() == m_listener)
return true;
- else
- return false;
+
+ return false;
}
private:
@@ -413,32 +412,30 @@ public:
}
/// Restore the state of the Broadcaster from a previous hijack attempt.
- ///
void RestoreBroadcaster() { m_broadcaster_sp->RestoreBroadcaster(); }
- // This needs to be filled in if you are going to register the broadcaster
- // with the broadcaster manager and do broadcaster class matching.
- // FIXME: Probably should make a ManagedBroadcaster subclass with all the
bits
- // needed to work
- // with the BroadcasterManager, so that it is clearer how to add one.
+ /// This needs to be filled in if you are going to register the broadcaster
+ /// with the broadcaster manager and do broadcaster class matching.
+ /// FIXME: Probably should make a ManagedBroadcaster subclass with all the
+ /// bits needed to work with the BroadcasterManager, so that it is clearer
+ /// how to add one.
virtual ConstString &GetBroadcasterClass() const;
lldb::BroadcasterManagerSP GetManager();
protected:
- // BroadcasterImpl contains the actual Broadcaster implementation. The
- // Broadcaster makes a BroadcasterImpl which lives as long as it does. The
- // Listeners & the Events hold a weak pointer to the BroadcasterImpl, so that
- // they can survive if a Broadcaster they were listening to is destroyed w/o
- // their being able to unregister from it (which can happen if the
- // Broadcasters & Listeners are being destroyed on separate threads
- // simultaneously. The Broadcaster itself can't be shared out as a weak
- // pointer, because some things that are broadcasters (e.g. the Target and
- // the Process) are shared in their own right.
- //
- // For the most part, the Broadcaster functions dispatch to the
- // BroadcasterImpl, and are documented in the public Broadcaster API above.
-
+ /// BroadcasterImpl contains the actual Broadcaster implementation. The
+ /// Broadcaster makes a BroadcasterImpl which lives as long as it does. The
+ /// Listeners & the Events hold a weak pointer to the BroadcasterImpl, so
+ /// that they can survive if a Broadcaster they were listening to is
+ /// destroyed w/o their being able to unregister from it (which can happen if
+ /// the Broadcasters & Listeners are being destroyed on separate threads
+ /// simultaneously. The Broadcaster itself can't be shared out as a weak
+ /// pointer, because some things that are broadcasters (e.g. the Target and
+ /// the Process) are shared in their own right.
+ ///
+ /// For the most part, the Broadcaster functions dispatch to the
+ /// BroadcasterImpl, and are documented in the public Broadcaster API above.
class BroadcasterImpl {
friend class Listener;
friend class Broadcaster;
@@ -505,7 +502,6 @@ protected:
const char *GetHijackingListenerName();
- //
typedef llvm::SmallVector<std::pair<lldb::ListenerWP, uint32_t>, 4>
collection;
typedef std::map<uint32_t, std::string> event_names_map;
@@ -513,22 +509,28 @@ protected:
llvm::SmallVector<std::pair<lldb::ListenerSP, uint32_t &>, 4>
GetListeners();
- Broadcaster &m_broadcaster; ///< The broadcaster that this implements
- event_names_map m_event_names; ///< Optionally define event names for
- ///readability and logging for each event
bit
- collection m_listeners; ///< A list of Listener / event_mask pairs that are
- ///listening to this broadcaster.
- std::recursive_mutex
- m_listeners_mutex; ///< A mutex that protects \a m_listeners.
- std::vector<lldb::ListenerSP> m_hijacking_listeners; // A simple mechanism
- // to intercept events
- // from a broadcaster
- std::vector<uint32_t> m_hijacking_masks; // At some point we may want to
- // have a stack or Listener
- // collections, but for now this is just for private hijacking.
+ /// The broadcaster that this implements.
+ Broadcaster &m_broadcaster;
+
+ /// Optionally define event names for readability and logging for each
+ /// event bit.
+ event_names_map m_event_names;
+
+ /// A list of Listener / event_mask pairs that are listening to this
+ /// broadcaster.
+ collection m_listeners;
+
+ /// A mutex that protects \a m_listeners.
+ std::recursive_mutex m_listeners_mutex;
+
+ /// A simple mechanism to intercept events from a broadcaster
+ std::vector<lldb::ListenerSP> m_hijacking_listeners;
+
+ /// At some point we may want to have a stack or Listener collections, but
+ /// for now this is just for private hijacking.
+ std::vector<uint32_t> m_hijacking_masks;
private:
- // For Broadcaster only
DISALLOW_COPY_AND_ASSIGN(BroadcasterImpl);
};
@@ -540,14 +542,13 @@ protected:
const char *GetHijackingListenerName() {
return m_broadcaster_sp->GetHijackingListenerName();
}
- // Classes that inherit from Broadcaster can see and modify these
private:
- // For Broadcaster only
BroadcasterImplSP m_broadcaster_sp;
lldb::BroadcasterManagerSP m_manager_sp;
- const ConstString
- m_broadcaster_name; ///< The name of this broadcaster object.
+
+ /// The name of this broadcaster object.
+ const ConstString m_broadcaster_name;
DISALLOW_COPY_AND_ASSIGN(Broadcaster);
};
Modified: lldb/trunk/source/Utility/Broadcaster.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/Broadcaster.cpp?rev=361597&r1=361596&r2=361597&view=diff
==============================================================================
--- lldb/trunk/source/Utility/Broadcaster.cpp (original)
+++ lldb/trunk/source/Utility/Broadcaster.cpp Thu May 23 21:41:47 2019
@@ -30,9 +30,8 @@ Broadcaster::Broadcaster(BroadcasterMana
: m_broadcaster_sp(std::make_shared<BroadcasterImpl>(*this)),
m_manager_sp(std::move(manager_sp)), m_broadcaster_name(name) {
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
- if (log)
- log->Printf("%p Broadcaster::Broadcaster(\"%s\")",
- static_cast<void *>(this), GetBroadcasterName().AsCString());
+ LLDB_LOG(log, "{0} Broadcaster::Broadcaster(\"{1}\")",
+ static_cast<void *>(this), GetBroadcasterName().AsCString());
}
Broadcaster::BroadcasterImpl::BroadcasterImpl(Broadcaster &broadcaster)
@@ -41,9 +40,8 @@ Broadcaster::BroadcasterImpl::Broadcaste
Broadcaster::~Broadcaster() {
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
- if (log)
- log->Printf("%p Broadcaster::~Broadcaster(\"%s\")",
- static_cast<void *>(this), m_broadcaster_name.AsCString());
+ LLDB_LOG(log, "{0} Broadcaster::~Broadcaster(\"{1}\")",
+ static_cast<void *>(this), GetBroadcasterName().AsCString());
Clear();
}
@@ -213,8 +211,7 @@ void Broadcaster::BroadcasterImpl::Priva
hijacking_listener_sp.reset();
}
- Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_EVENTS));
- if (log) {
+ if (Log *log = lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_EVENTS)) {
StreamString event_description;
event_sp->Dump(&event_description);
log->Printf("%p Broadcaster(\"%s\")::BroadcastEvent (event_sp = {%s}, "
@@ -225,18 +222,16 @@ void Broadcaster::BroadcasterImpl::Priva
}
if (hijacking_listener_sp) {
- if (unique &&
- hijacking_listener_sp->PeekAtNextEventForBroadcasterWithType(
- &m_broadcaster, event_type))
+ if (unique && hijacking_listener_sp->PeekAtNextEventForBroadcasterWithType(
+ &m_broadcaster, event_type))
return;
hijacking_listener_sp->AddEvent(event_sp);
} else {
for (auto &pair : GetListeners()) {
if (!(pair.second & event_type))
continue;
- if (unique &&
- pair.first->PeekAtNextEventForBroadcasterWithType(&m_broadcaster,
- event_type))
+ if (unique && pair.first->PeekAtNextEventForBroadcasterWithType(
+ &m_broadcaster, event_type))
continue;
pair.first->AddEvent(event_sp);
@@ -267,11 +262,11 @@ bool Broadcaster::BroadcasterImpl::Hijac
std::lock_guard<std::recursive_mutex> guard(m_listeners_mutex);
Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_EVENTS));
- if (log)
- log->Printf(
- "%p Broadcaster(\"%s\")::HijackBroadcaster (listener(\"%s\")=%p)",
- static_cast<void *>(this), GetBroadcasterName(),
- listener_sp->m_name.c_str(), static_cast<void *>(listener_sp.get()));
+ LLDB_LOG(
+ log,
+ "{0} Broadcaster(\"{1}\")::HijackBroadcaster (listener(\"{2}\")={3})",
+ static_cast<void *>(this), GetBroadcasterName(),
+ listener_sp->m_name.c_str(), static_cast<void *>(listener_sp.get()));
m_hijacking_listeners.push_back(listener_sp);
m_hijacking_masks.push_back(event_mask);
return true;
@@ -288,24 +283,22 @@ bool Broadcaster::BroadcasterImpl::IsHij
const char *Broadcaster::BroadcasterImpl::GetHijackingListenerName() {
if (m_hijacking_listeners.size()) {
return m_hijacking_listeners.back()->GetName();
- } else {
- return nullptr;
}
+ return nullptr;
}
void Broadcaster::BroadcasterImpl::RestoreBroadcaster() {
std::lock_guard<std::recursive_mutex> guard(m_listeners_mutex);
if (!m_hijacking_listeners.empty()) {
+ ListenerSP listener_sp = m_hijacking_listeners.back();
Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_EVENTS));
- if (log) {
- ListenerSP listener_sp = m_hijacking_listeners.back();
- log->Printf("%p Broadcaster(\"%s\")::RestoreBroadcaster (about to pop "
- "listener(\"%s\")=%p)",
- static_cast<void *>(this), GetBroadcasterName(),
- listener_sp->m_name.c_str(),
- static_cast<void *>(listener_sp.get()));
- }
+ LLDB_LOG(log,
+ "{0} Broadcaster(\"{1}\")::RestoreBroadcaster (about to pop "
+ "listener(\"{2}\")={3})",
+ static_cast<void *>(this), GetBroadcasterName(),
+ listener_sp->m_name.c_str(),
+ static_cast<void *>(listener_sp.get()));
m_hijacking_listeners.pop_back();
}
if (!m_hijacking_masks.empty())
@@ -320,9 +313,8 @@ ConstString &Broadcaster::GetBroadcaster
bool BroadcastEventSpec::operator<(const BroadcastEventSpec &rhs) const {
if (GetBroadcasterClass() == rhs.GetBroadcasterClass()) {
return GetEventBits() < rhs.GetEventBits();
- } else {
- return GetBroadcasterClass() < rhs.GetBroadcasterClass();
}
+ return GetBroadcasterClass() < rhs.GetBroadcasterClass();
}
BroadcastEventSpec &BroadcastEventSpec::
@@ -378,17 +370,16 @@ bool BroadcasterManager::UnregisterListe
iter = find_if(m_event_map.begin(), end_iter, predicate);
if (iter == end_iter) {
break;
- } else {
- uint32_t iter_event_bits = (*iter).first.GetEventBits();
- removed_some = true;
-
- if (event_bits_to_remove != iter_event_bits) {
- uint32_t new_event_bits = iter_event_bits & ~event_bits_to_remove;
- to_be_readded.push_back(BroadcastEventSpec(
- event_spec.GetBroadcasterClass(), new_event_bits));
- }
- m_event_map.erase(iter);
}
+ uint32_t iter_event_bits = (*iter).first.GetEventBits();
+ removed_some = true;
+
+ if (event_bits_to_remove != iter_event_bits) {
+ uint32_t new_event_bits = iter_event_bits & ~event_bits_to_remove;
+ to_be_readded.push_back(
+ BroadcastEventSpec(event_spec.GetBroadcasterClass(),
new_event_bits));
+ }
+ m_event_map.erase(iter);
}
// Okay now add back the bits that weren't completely removed:
@@ -408,8 +399,8 @@ ListenerSP BroadcasterManager::GetListen
BroadcastEventSpecMatches(event_spec));
if (iter != end_iter)
return (*iter).second;
- else
- return nullptr;
+
+ return nullptr;
}
void BroadcasterManager::RemoveListener(Listener *listener) {
@@ -427,8 +418,8 @@ void BroadcasterManager::RemoveListener(
iter = find_if(m_event_map.begin(), end_iter, predicate);
if (iter == end_iter)
break;
- else
- m_event_map.erase(iter);
+
+ m_event_map.erase(iter);
}
}
@@ -444,8 +435,8 @@ void BroadcasterManager::RemoveListener(
iter = find_if(m_event_map.begin(), end_iter, predicate);
if (iter == end_iter)
break;
- else
- m_event_map.erase(iter);
+
+ m_event_map.erase(iter);
}
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
