GD L2cap improvements

* Store the role from updates.
* Update PSM check.  LE doesn't need it because it uses SPSM.

Tag: #gd-refactor
Bug: 141555841
Test: cert/run --host
Change-Id: Ibfea3d227a999c8e31e6897628c257084b43174e

diff --git a/gd/l2cap/classic/internal/link.cc b/gd/l2cap/classic/internal/link.cc
index f66aae2..90664d0 100644 (file)
--- a/gd/l2cap/classic/internal/link.cc
+++ b/gd/l2cap/classic/internal/link.cc
@@ -402,7 +402,7 @@ void Link::OnCentralLinkKeyComplete(hci::KeyFlag key_flag) {
   LOG_INFO("UNIMPLEMENTED key_flag:%s", hci::KeyFlagText(key_flag).c_str());
 }
 void Link::OnRoleChange(hci::Role new_role) {
-  LOG_INFO("UNIMPLEMENTED role:%s", hci::RoleText(new_role).c_str());
+  role_ = new_role;
 }
 void Link::OnDisconnection(hci::ErrorCode reason) {
   OnAclDisconnected(reason);

diff --git a/gd/l2cap/classic/internal/link_manager.cc b/gd/l2cap/classic/internal/link_manager.cc
index 5f0bda9..86cd820 100644 (file)
--- a/gd/l2cap/classic/internal/link_manager.cc
+++ b/gd/l2cap/classic/internal/link_manager.cc
@@ -83,6 +83,9 @@ void LinkManager::ConnectFixedChannelServices(hci::Address device,
 
 void LinkManager::ConnectDynamicChannelServices(
     hci::Address device, Link::PendingDynamicChannelConnection pending_dynamic_channel_connection, Psm psm) {
+  if (!IsPsmValid(psm)) {
+    return;
+  }
   auto* link = GetLink(device);
   if (link == nullptr) {
     acl_manager_->CreateConnection(device);

diff --git a/gd/l2cap/classic/l2cap_classic_module.h b/gd/l2cap/classic/l2cap_classic_module.h
index 1e6a100..4ce7db6 100644 (file)
--- a/gd/l2cap/classic/l2cap_classic_module.h
+++ b/gd/l2cap/classic/l2cap_classic_module.h
@@ -63,6 +63,8 @@ class L2capClassicModule : public bluetooth::Module {
    */
   virtual SecurityInterface* GetSecurityInterface(os::Handler* handler, LinkSecurityInterfaceListener* listener);
 
+  friend security::SecurityModule;
+
  protected:
   void ListDependencies(ModuleList* list) override;
 
@@ -78,8 +80,6 @@ class L2capClassicModule : public bluetooth::Module {
   struct impl;
   std::unique_ptr<impl> pimpl_;
 
-  friend security::SecurityModule;
-
   DISALLOW_COPY_AND_ASSIGN(L2capClassicModule);
 };
 

diff --git a/gd/l2cap/internal/dynamic_channel_allocator.cc b/gd/l2cap/internal/dynamic_channel_allocator.cc
index 761a55c..1911b2a 100644 (file)
--- a/gd/l2cap/internal/dynamic_channel_allocator.cc
+++ b/gd/l2cap/internal/dynamic_channel_allocator.cc
@@ -28,8 +28,6 @@ namespace l2cap {
 namespace internal {
 
 std::shared_ptr<DynamicChannelImpl> DynamicChannelAllocator::AllocateChannel(Psm psm, Cid remote_cid) {
-  ASSERT_LOG(IsPsmValid(psm), "Psm 0x%x is invalid", psm);
-
   if (used_remote_cid_.find(remote_cid) != used_remote_cid_.end()) {
     LOG_INFO("Remote cid 0x%x is used", remote_cid);
     return nullptr;
@@ -54,8 +52,6 @@ std::shared_ptr<DynamicChannelImpl> DynamicChannelAllocator::AllocateChannel(Psm
 
 std::shared_ptr<DynamicChannelImpl> DynamicChannelAllocator::AllocateReservedChannel(Cid reserved_cid, Psm psm,
                                                                                      Cid remote_cid) {
-  ASSERT_LOG(IsPsmValid(psm), "Psm 0x%x is invalid", psm);
-
   if (used_remote_cid_.find(remote_cid) != used_remote_cid_.end()) {
     LOG_INFO("Remote cid 0x%x is used", remote_cid);
     return nullptr;

diff --git a/gd/l2cap/internal/dynamic_channel_impl.cc b/gd/l2cap/internal/dynamic_channel_impl.cc
index 372f753..6a3ecc5 100644 (file)
--- a/gd/l2cap/internal/dynamic_channel_impl.cc
+++ b/gd/l2cap/internal/dynamic_channel_impl.cc
@@ -33,7 +33,6 @@ DynamicChannelImpl::DynamicChannelImpl(Psm psm, Cid cid, Cid remote_cid, l2cap::
                                        os::Handler* l2cap_handler)
     : psm_(psm), cid_(cid), remote_cid_(remote_cid), link_(link), l2cap_handler_(l2cap_handler),
       device_(link->GetDevice()) {
-  ASSERT(IsPsmValid(psm_));
   ASSERT(cid_ > 0);
   ASSERT(remote_cid_ > 0);
   ASSERT(link_ != nullptr);

diff --git a/gd/l2cap/le/dynamic_channel_service.h b/gd/l2cap/le/dynamic_channel_service.h
index 408a20c..898c885 100644 (file)
--- a/gd/l2cap/le/dynamic_channel_service.h
+++ b/gd/l2cap/le/dynamic_channel_service.h
@@ -50,7 +50,6 @@ class DynamicChannelService {
  protected:
   DynamicChannelService(Psm psm, internal::DynamicChannelServiceManagerImpl* manager, os::Handler* handler)
       : psm_(psm), manager_(manager), l2cap_layer_handler_(handler) {
-    ASSERT(IsPsmValid(psm));
     ASSERT(manager_ != nullptr);
     ASSERT(l2cap_layer_handler_ != nullptr);
   }

diff --git a/gd/l2cap/le/internal/dynamic_channel_service_manager_impl.cc b/gd/l2cap/le/internal/dynamic_channel_service_manager_impl.cc
index 2c715ca..4e848f4 100644 (file)
--- a/gd/l2cap/le/internal/dynamic_channel_service_manager_impl.cc
+++ b/gd/l2cap/le/internal/dynamic_channel_service_manager_impl.cc
@@ -28,12 +28,7 @@ namespace internal {
 
 void DynamicChannelServiceManagerImpl::Register(Psm psm,
                                                 DynamicChannelServiceImpl::PendingRegistration pending_registration) {
-  if (!IsPsmValid(psm)) {
-    std::unique_ptr<DynamicChannelService> invalid_service(new DynamicChannelService());
-    pending_registration.user_handler_->Post(
-        common::BindOnce(std::move(pending_registration.on_registration_complete_callback_),
-                         DynamicChannelManager::RegistrationResult::FAIL_INVALID_SERVICE, std::move(invalid_service)));
-  } else if (IsServiceRegistered(psm)) {
+  if (IsServiceRegistered(psm)) {
     std::unique_ptr<DynamicChannelService> invalid_service(new DynamicChannelService());
     pending_registration.user_handler_->Post(common::BindOnce(
         std::move(pending_registration.on_registration_complete_callback_),

diff --git a/gd/l2cap/le/internal/dynamic_channel_service_manager_test.cc b/gd/l2cap/le/internal/dynamic_channel_service_manager_test.cc
index 25ea136..874db82 100644 (file)
--- a/gd/l2cap/le/internal/dynamic_channel_service_manager_test.cc
+++ b/gd/l2cap/le/internal/dynamic_channel_service_manager_test.cc
@@ -79,28 +79,28 @@ TEST_F(L2capLeDynamicServiceManagerTest, register_and_unregister_le_dynamic_chan
       .security_policy_ = SecurityPolicy::NO_SECURITY_WHATSOEVER_PLAINTEXT_TRANSPORT_OK,
       .on_registration_complete_callback_ =
           common::BindOnce(&L2capLeDynamicServiceManagerTest::OnServiceRegistered, common::Unretained(this), true)};
-  Cid cid = kSmpBrCid;
-  EXPECT_FALSE(manager_->IsServiceRegistered(cid));
-  manager_->Register(cid, std::move(pending_registration));
-  EXPECT_TRUE(manager_->IsServiceRegistered(cid));
+  Psm psm = 0x41;
+  EXPECT_FALSE(manager_->IsServiceRegistered(psm));
+  manager_->Register(psm, std::move(pending_registration));
+  EXPECT_TRUE(manager_->IsServiceRegistered(psm));
   sync_user_handler();
   EXPECT_TRUE(service_registered_);
-  manager_->Unregister(cid, common::BindOnce([] {}), user_handler_);
-  EXPECT_FALSE(manager_->IsServiceRegistered(cid));
+  manager_->Unregister(psm, common::BindOnce([] {}), user_handler_);
+  EXPECT_FALSE(manager_->IsServiceRegistered(psm));
 }
 
-TEST_F(L2capLeDynamicServiceManagerTest, register_le_dynamic_channel_bad_cid) {
+TEST_F(L2capLeDynamicServiceManagerTest, register_le_dynamic_channel_even_number_psm) {
   DynamicChannelServiceImpl::PendingRegistration pending_registration{
       .user_handler_ = user_handler_,
       .security_policy_ = SecurityPolicy::NO_SECURITY_WHATSOEVER_PLAINTEXT_TRANSPORT_OK,
       .on_registration_complete_callback_ =
-          common::BindOnce(&L2capLeDynamicServiceManagerTest::OnServiceRegistered, common::Unretained(this), false)};
-  Cid cid = 0x1000;
-  EXPECT_FALSE(manager_->IsServiceRegistered(cid));
-  manager_->Register(cid, std::move(pending_registration));
-  EXPECT_FALSE(manager_->IsServiceRegistered(cid));
+          common::BindOnce(&L2capLeDynamicServiceManagerTest::OnServiceRegistered, common::Unretained(this), true)};
+  Psm psm = 0x0100;
+  EXPECT_FALSE(manager_->IsServiceRegistered(psm));
+  manager_->Register(psm, std::move(pending_registration));
+  EXPECT_TRUE(manager_->IsServiceRegistered(psm));
   sync_user_handler();
-  EXPECT_FALSE(service_registered_);
+  EXPECT_TRUE(service_registered_);
 }
 
 }  // namespace internal

diff --git a/gd/l2cap/le/internal/signalling_manager.cc b/gd/l2cap/le/internal/signalling_manager.cc
index ac454c2..44309c5 100644 (file)
--- a/gd/l2cap/le/internal/signalling_manager.cc
+++ b/gd/l2cap/le/internal/signalling_manager.cc
@@ -173,13 +173,6 @@ void LeSignallingManager::OnConnectionParameterUpdateResponse(SignalId signal_id
 
 void LeSignallingManager::OnConnectionRequest(SignalId signal_id, Psm psm, Cid remote_cid, Mtu mtu, uint16_t mps,
                                               uint16_t initial_credits) {
-  if (!IsPsmValid(psm)) {
-    LOG_WARN("Invalid psm received from remote psm:%d remote_cid:%d", psm, remote_cid);
-    send_connection_response(signal_id, kInvalidCid, 0, 0, 0,
-                             LeCreditBasedConnectionResponseResult::LE_PSM_NOT_SUPPORTED);
-    return;
-  }
-
   if (remote_cid == kInvalidCid) {
     LOG_WARN("Invalid remote cid received from remote psm:%d remote_cid:%d", psm, remote_cid);
     send_connection_response(signal_id, kInvalidCid, 0, 0, 0,