[bt][data] SocketChannelRelay: remove a template argument
Instead of having SocketChannelRelay provide the channel's identifier
to the SocketFactory when invoking the DeactivationCallback, have the
factory capture the identifier into the callback. This eliminates the
need for the SocketChannelRelay to know about ChannelIdT.
Bug: NET-1932
Test: bt-host-unittests (on eve)
Change-Id: I9ef4505a1828ced9653849114e5d2e6a3517fd7d
diff --git a/drivers/bluetooth/lib/data/l2cap_socket_channel_relay.cc b/drivers/bluetooth/lib/data/l2cap_socket_channel_relay.cc
index b7ff99e..f2f9268 100644
--- a/drivers/bluetooth/lib/data/l2cap_socket_channel_relay.cc
+++ b/drivers/bluetooth/lib/data/l2cap_socket_channel_relay.cc
@@ -27,6 +27,5 @@
#include "garnet/drivers/bluetooth/lib/data/socket_channel_relay.cc"
namespace btlib::data::internal {
-template class SocketChannelRelay<l2cap::Channel, l2cap::Channel::UniqueId,
- l2cap::SDU>;
+template class SocketChannelRelay<l2cap::Channel, l2cap::SDU>;
} // namespace btlib::data::internal
diff --git a/drivers/bluetooth/lib/data/l2cap_socket_channel_relay.h b/drivers/bluetooth/lib/data/l2cap_socket_channel_relay.h
index b2b95d3..c9245ac 100644
--- a/drivers/bluetooth/lib/data/l2cap_socket_channel_relay.h
+++ b/drivers/bluetooth/lib/data/l2cap_socket_channel_relay.h
@@ -11,8 +11,7 @@
#include "garnet/drivers/bluetooth/lib/l2cap/sdu.h"
namespace btlib::data::internal {
-using L2capSocketChannelRelay =
- SocketChannelRelay<l2cap::Channel, l2cap::Channel::UniqueId, l2cap::SDU>;
+using L2capSocketChannelRelay = SocketChannelRelay<l2cap::Channel, l2cap::SDU>;
} // namespace btlib::data::internal
#endif // GARNET_DRIVERS_BLUETOOTH_LIB_DATA_L2CAP_SOCKET_CHANNEL_RELAY_H_
diff --git a/drivers/bluetooth/lib/data/rfcomm_socket_channel_relay.cc b/drivers/bluetooth/lib/data/rfcomm_socket_channel_relay.cc
index 33f0d6c..bb50f1e 100644
--- a/drivers/bluetooth/lib/data/rfcomm_socket_channel_relay.cc
+++ b/drivers/bluetooth/lib/data/rfcomm_socket_channel_relay.cc
@@ -28,6 +28,5 @@
#include "garnet/drivers/bluetooth/lib/data/socket_channel_relay.cc"
namespace btlib::data::internal {
-template class SocketChannelRelay<rfcomm::Channel, rfcomm::Channel::UniqueId,
- common::ByteBufferPtr>;
+template class SocketChannelRelay<rfcomm::Channel, common::ByteBufferPtr>;
} // namespace btlib::data::internal
diff --git a/drivers/bluetooth/lib/data/rfcomm_socket_channel_relay.h b/drivers/bluetooth/lib/data/rfcomm_socket_channel_relay.h
index 9e9d5e4..296a1ea 100644
--- a/drivers/bluetooth/lib/data/rfcomm_socket_channel_relay.h
+++ b/drivers/bluetooth/lib/data/rfcomm_socket_channel_relay.h
@@ -12,8 +12,7 @@
namespace btlib::data::internal {
using RfcommSocketChannelRelay =
- SocketChannelRelay<rfcomm::Channel, rfcomm::Channel::UniqueId,
- common::ByteBufferPtr>;
+ SocketChannelRelay<rfcomm::Channel, common::ByteBufferPtr>;
} // namespace btlib::data::internal
#endif // GARNET_DRIVERS_BLUETOOTH_LIB_DATA_RFCOMM_SOCKET_CHANNEL_RELAY_H_
diff --git a/drivers/bluetooth/lib/data/socket_channel_relay.cc b/drivers/bluetooth/lib/data/socket_channel_relay.cc
index 64e301f..b7662c1 100644
--- a/drivers/bluetooth/lib/data/socket_channel_relay.cc
+++ b/drivers/bluetooth/lib/data/socket_channel_relay.cc
@@ -14,8 +14,8 @@
namespace btlib::data::internal {
-template <typename ChannelT, typename ChannelIdT, typename RxDataT>
-SocketChannelRelay<ChannelT, ChannelIdT, RxDataT>::SocketChannelRelay(
+template <typename ChannelT, typename RxDataT>
+SocketChannelRelay<ChannelT, RxDataT>::SocketChannelRelay(
zx::socket socket, fbl::RefPtr<ChannelT> channel,
DeactivationCallback deactivation_cb)
: state_(RelayState::kActivating),
@@ -39,8 +39,8 @@
fit::bind_member(this, &SocketChannelRelay::OnSocketClosed));
}
-template <typename ChannelT, typename ChannelIdT, typename RxDataT>
-SocketChannelRelay<ChannelT, ChannelIdT, RxDataT>::~SocketChannelRelay() {
+template <typename ChannelT, typename RxDataT>
+SocketChannelRelay<ChannelT, RxDataT>::~SocketChannelRelay() {
ZX_DEBUG_ASSERT(thread_checker_.IsCreationThreadCurrent());
if (state_ != RelayState::kDeactivated) {
@@ -52,8 +52,8 @@
}
}
-template <typename ChannelT, typename ChannelIdT, typename RxDataT>
-bool SocketChannelRelay<ChannelT, ChannelIdT, RxDataT>::Activate() {
+template <typename ChannelT, typename RxDataT>
+bool SocketChannelRelay<ChannelT, RxDataT>::Activate() {
ZX_DEBUG_ASSERT(state_ == RelayState::kActivating);
// Note: we assume that BeginWait() does not synchronously dispatch any
@@ -100,8 +100,8 @@
return true;
}
-template <typename ChannelT, typename ChannelIdT, typename RxDataT>
-void SocketChannelRelay<ChannelT, ChannelIdT, RxDataT>::Deactivate() {
+template <typename ChannelT, typename RxDataT>
+void SocketChannelRelay<ChannelT, RxDataT>::Deactivate() {
ZX_DEBUG_ASSERT(state_ != RelayState::kDeactivated);
state_ = RelayState::kDeactivating;
@@ -127,21 +127,20 @@
state_ = RelayState::kDeactivated;
}
-template <typename ChannelT, typename ChannelIdT, typename RxDataT>
-void SocketChannelRelay<ChannelT, ChannelIdT,
- RxDataT>::DeactivateAndRequestDestruction() {
+template <typename ChannelT, typename RxDataT>
+void SocketChannelRelay<ChannelT, RxDataT>::DeactivateAndRequestDestruction() {
Deactivate();
if (deactivation_cb_) {
// NOTE: deactivation_cb_ is expected to destroy |this|. Since |this|
// owns deactivation_cb_, we move() deactivation_cb_ outside of |this|
// before invoking the callback.
auto moved_deactivation_cb = std::move(deactivation_cb_);
- moved_deactivation_cb(channel_->unique_id());
+ moved_deactivation_cb();
}
}
-template <typename ChannelT, typename ChannelIdT, typename RxDataT>
-void SocketChannelRelay<ChannelT, ChannelIdT, RxDataT>::OnSocketReadable(
+template <typename ChannelT, typename RxDataT>
+void SocketChannelRelay<ChannelT, RxDataT>::OnSocketReadable(
zx_status_t status) {
ZX_DEBUG_ASSERT(state_ == RelayState::kActivated);
if (!CopyFromSocketToChannel() ||
@@ -150,23 +149,22 @@
}
}
-template <typename ChannelT, typename ChannelIdT, typename RxDataT>
-void SocketChannelRelay<ChannelT, ChannelIdT, RxDataT>::OnSocketWritable(
+template <typename ChannelT, typename RxDataT>
+void SocketChannelRelay<ChannelT, RxDataT>::OnSocketWritable(
zx_status_t status) {
ZX_DEBUG_ASSERT(state_ == RelayState::kActivated);
ZX_DEBUG_ASSERT(!socket_write_queue_.empty());
ServiceSocketWriteQueue();
}
-template <typename ChannelT, typename ChannelIdT, typename RxDataT>
-void SocketChannelRelay<ChannelT, ChannelIdT, RxDataT>::OnSocketClosed(
- zx_status_t status) {
+template <typename ChannelT, typename RxDataT>
+void SocketChannelRelay<ChannelT, RxDataT>::OnSocketClosed(zx_status_t status) {
ZX_DEBUG_ASSERT(state_ == RelayState::kActivated);
DeactivateAndRequestDestruction();
}
-template <typename ChannelT, typename ChannelIdT, typename RxDataT>
-void SocketChannelRelay<ChannelT, ChannelIdT, RxDataT>::OnChannelDataReceived(
+template <typename ChannelT, typename RxDataT>
+void SocketChannelRelay<ChannelT, RxDataT>::OnChannelDataReceived(
RxDataT rx_data) {
ZX_DEBUG_ASSERT(thread_checker_.IsCreationThreadCurrent());
// Note: kActivating is deliberately permitted, as ChannelImpl::Activate()
@@ -184,8 +182,8 @@
ServiceSocketWriteQueue();
}
-template <typename ChannelT, typename ChannelIdT, typename RxDataT>
-void SocketChannelRelay<ChannelT, ChannelIdT, RxDataT>::OnChannelClosed() {
+template <typename ChannelT, typename RxDataT>
+void SocketChannelRelay<ChannelT, RxDataT>::OnChannelClosed() {
ZX_DEBUG_ASSERT(thread_checker_.IsCreationThreadCurrent());
ZX_DEBUG_ASSERT(state_ != RelayState::kActivating);
ZX_DEBUG_ASSERT(state_ != RelayState::kDeactivated);
@@ -204,9 +202,8 @@
DeactivateAndRequestDestruction();
}
-template <typename ChannelT, typename ChannelIdT, typename RxDataT>
-bool SocketChannelRelay<ChannelT, ChannelIdT,
- RxDataT>::CopyFromSocketToChannel() {
+template <typename ChannelT, typename RxDataT>
+bool SocketChannelRelay<ChannelT, RxDataT>::CopyFromSocketToChannel() {
// Subtle: we make the read buffer larger than the TX MTU, so that we can
// detect truncated datagrams.
const size_t read_buf_size = channel_->tx_mtu() + 1;
@@ -257,9 +254,8 @@
return true;
}
-template <typename ChannelT, typename ChannelIdT, typename RxDataT>
-void SocketChannelRelay<ChannelT, ChannelIdT,
- RxDataT>::ServiceSocketWriteQueue() {
+template <typename ChannelT, typename RxDataT>
+void SocketChannelRelay<ChannelT, RxDataT>::ServiceSocketWriteQueue() {
// TODO(NET-1477): Similarly to CopyFromSocketToChannel(), we may want to
// consider yielding occasionally. The data-rate from the Channel into the
// socket write queue should be bounded by PHY layer data rates, which are
@@ -327,8 +323,8 @@
}
}
-template <typename ChannelT, typename ChannelIdT, typename RxDataT>
-void SocketChannelRelay<ChannelT, ChannelIdT, RxDataT>::BindWait(
+template <typename ChannelT, typename RxDataT>
+void SocketChannelRelay<ChannelT, RxDataT>::BindWait(
zx_signals_t trigger, const char* wait_name, async::Wait* wait,
fit::function<void(zx_status_t)> handler) {
wait->set_object(socket_.get());
@@ -374,9 +370,9 @@
});
}
-template <typename ChannelT, typename ChannelIdT, typename RxDataT>
-bool SocketChannelRelay<ChannelT, ChannelIdT, RxDataT>::BeginWait(
- const char* wait_name, async::Wait* wait) {
+template <typename ChannelT, typename RxDataT>
+bool SocketChannelRelay<ChannelT, RxDataT>::BeginWait(const char* wait_name,
+ async::Wait* wait) {
ZX_DEBUG_ASSERT(state_ != RelayState::kDeactivating);
ZX_DEBUG_ASSERT(state_ != RelayState::kDeactivated);
@@ -396,8 +392,8 @@
return true;
}
-template <typename ChannelT, typename ChannelIdT, typename RxDataT>
-void SocketChannelRelay<ChannelT, ChannelIdT, RxDataT>::UnbindAndCancelWait(
+template <typename ChannelT, typename RxDataT>
+void SocketChannelRelay<ChannelT, RxDataT>::UnbindAndCancelWait(
async::Wait* wait) {
ZX_DEBUG_ASSERT(state_ != RelayState::kActivating);
ZX_DEBUG_ASSERT(state_ != RelayState::kDeactivated);
diff --git a/drivers/bluetooth/lib/data/socket_channel_relay.h b/drivers/bluetooth/lib/data/socket_channel_relay.h
index a71b2ad..cc90050 100644
--- a/drivers/bluetooth/lib/data/socket_channel_relay.h
+++ b/drivers/bluetooth/lib/data/socket_channel_relay.h
@@ -25,10 +25,10 @@
// THREAD-SAFETY: This class is thread-hostile. Creation, use, and destruction
// _must_ occur on a single thread. |dispatcher|, which _must_ be
// single-threaded, must run on that same thread.
-template <typename ChannelT, typename ChannelIdT, typename RxDataT>
+template <typename ChannelT, typename RxDataT>
class SocketChannelRelay final {
public:
- using DeactivationCallback = fit::function<void(ChannelIdT)>;
+ using DeactivationCallback = fit::function<void()>;
// Creates a SocketChannelRelay which executes on |dispatcher|. Note that
// |dispatcher| must be single-threaded.
diff --git a/drivers/bluetooth/lib/data/socket_channel_relay_unittest.cc b/drivers/bluetooth/lib/data/socket_channel_relay_unittest.cc
index f68ff11..4604916 100644
--- a/drivers/bluetooth/lib/data/socket_channel_relay_unittest.cc
+++ b/drivers/bluetooth/lib/data/socket_channel_relay_unittest.cc
@@ -23,9 +23,7 @@
// We'll test the template using (only) the set of type parameters necessary for
// the L2CAP instantiation.
-using RelayT =
- internal::SocketChannelRelay<l2cap::Channel, l2cap::Channel::UniqueId,
- l2cap::SDU>;
+using RelayT = internal::SocketChannelRelay<l2cap::Channel, l2cap::SDU>;
static_assert(std::is_same_v<RelayT, internal::L2capSocketChannelRelay>);
class DATA_SocketChannelRelayTest : public ::testing::Test {
@@ -136,9 +134,8 @@
DATA_SocketChannelRelayLifetimeTest()
: was_deactivation_callback_invoked_(false),
relay_(std::make_unique<RelayT>(
- ConsumeLocalSocket(), channel(), [this](auto channel_id) {
- was_deactivation_callback_invoked_ = true;
- })) {}
+ ConsumeLocalSocket(), channel(),
+ [this]() { was_deactivation_callback_invoked_ = true; })) {}
protected:
bool was_deactivation_callback_invoked() {
diff --git a/drivers/bluetooth/lib/data/socket_factory.cc b/drivers/bluetooth/lib/data/socket_factory.cc
index faf5780..542a2d6 100644
--- a/drivers/bluetooth/lib/data/socket_factory.cc
+++ b/drivers/bluetooth/lib/data/socket_factory.cc
@@ -28,7 +28,8 @@
ZX_DEBUG_ASSERT(thread_checker_.IsCreationThreadCurrent());
ZX_DEBUG_ASSERT(channel);
- if (channel_to_relay_.find(channel->unique_id()) != channel_to_relay_.end()) {
+ const auto unique_id = channel->unique_id();
+ if (channel_to_relay_.find(unique_id) != channel_to_relay_.end()) {
bt_log(ERROR, "l2cap", "channel %u @ %u is already bound to a socket",
channel->link_handle(), channel->id());
return {};
@@ -46,12 +47,11 @@
auto relay = std::make_unique<RelayT>(
std::move(local_socket), channel,
typename RelayT::DeactivationCallback(
- [self =
- weak_ptr_factory_.GetWeakPtr()](ChannelIdT channel_id) mutable {
- ZX_DEBUG_ASSERT_MSG(self, "(unique_id=%lu)", channel_id);
- size_t n_erased = self->channel_to_relay_.erase(channel_id);
+ [self = weak_ptr_factory_.GetWeakPtr(), id = unique_id]() mutable {
+ ZX_DEBUG_ASSERT_MSG(self, "(unique_id=%lu)", id);
+ size_t n_erased = self->channel_to_relay_.erase(id);
ZX_DEBUG_ASSERT_MSG(n_erased == 1, "(n_erased=%zu, unique_id=%lu)",
- n_erased, channel_id);
+ n_erased, id);
}));
// Note: Activate() may abort, if |channel| has been Activated() without
@@ -62,7 +62,7 @@
return {};
}
- channel_to_relay_.emplace(channel->unique_id(), std::move(relay));
+ channel_to_relay_.emplace(unique_id, std::move(relay));
return remote_socket;
}
diff --git a/drivers/bluetooth/lib/data/socket_factory.h b/drivers/bluetooth/lib/data/socket_factory.h
index 20339b9..705a645 100644
--- a/drivers/bluetooth/lib/data/socket_factory.h
+++ b/drivers/bluetooth/lib/data/socket_factory.h
@@ -55,7 +55,7 @@
zx::socket MakeSocketForChannel(fbl::RefPtr<ChannelT> channel);
private:
- using RelayT = SocketChannelRelay<ChannelT, ChannelIdT, ChannelRxDataT>;
+ using RelayT = SocketChannelRelay<ChannelT, ChannelRxDataT>;
const fxl::ThreadChecker thread_checker_;
