From 725a7249a576c577b7cd379c0f5f4dccf72b0eee Mon Sep 17 00:00:00 2001
From: Todd Eisenberger <teisenbe@google.com>
Date: Mon, 22 Apr 2019 15:34:49 -0700
Subject: [PATCH] [devcoordinator] Finish encapsulating set_protocol_id

This was unblocked a while back by some other refactors

Change-Id: I36072fefda4165df3e5a5bb9a41d4d726fedafc5
---
 .../core/devmgr/devcoordinator/coordinator.cpp      | 11 ++++-------
 zircon/system/core/devmgr/devcoordinator/device.cpp | 13 +++++--------
 zircon/system/core/devmgr/devcoordinator/device.h   |  7 ++-----
 3 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/zircon/system/core/devmgr/devcoordinator/coordinator.cpp b/zircon/system/core/devmgr/devcoordinator/coordinator.cpp
index adfefcf31e9..652e8a0f80c 100644
--- a/zircon/system/core/devmgr/devcoordinator/coordinator.cpp
+++ b/zircon/system/core/devmgr/devcoordinator/coordinator.cpp
@@ -113,15 +113,14 @@ bool Coordinator::InSuspend() const {
 }
 
 zx_status_t Coordinator::InitializeCoreDevices(const char* sys_device_driver) {
-    root_device_ = fbl::MakeRefCounted<Device>(this, nullptr);
-    misc_device_ = fbl::MakeRefCounted<Device>(this, root_device_);
-    sys_device_ = fbl::MakeRefCounted<Device>(this, root_device_);
-    test_device_ = fbl::MakeRefCounted<Device>(this, root_device_);
+    root_device_ = fbl::MakeRefCounted<Device>(this, nullptr, ZX_PROTOCOL_ROOT);
+    misc_device_ = fbl::MakeRefCounted<Device>(this, root_device_, ZX_PROTOCOL_MISC_PARENT);
+    sys_device_ = fbl::MakeRefCounted<Device>(this, root_device_, 0);
+    test_device_ = fbl::MakeRefCounted<Device>(this, root_device_, ZX_PROTOCOL_TEST_PARENT);
 
     fbl::AllocChecker ac;
     {
         root_device_->flags = DEV_CTX_IMMORTAL | DEV_CTX_MUST_ISOLATE | DEV_CTX_MULTI_BIND;
-        root_device_->set_protocol_id(ZX_PROTOCOL_ROOT);
         root_device_->name = fbl::String("root", &ac);
         if (!ac.check()) {
             return ZX_ERR_NO_MEMORY;
@@ -134,7 +133,6 @@ zx_status_t Coordinator::InitializeCoreDevices(const char* sys_device_driver) {
 
     {
         misc_device_->flags = DEV_CTX_IMMORTAL | DEV_CTX_MUST_ISOLATE | DEV_CTX_MULTI_BIND;
-        misc_device_->set_protocol_id(ZX_PROTOCOL_MISC_PARENT);
         misc_device_->name = fbl::String("misc", &ac);
         if (!ac.check()) {
             return ZX_ERR_NO_MEMORY;
@@ -163,7 +161,6 @@ zx_status_t Coordinator::InitializeCoreDevices(const char* sys_device_driver) {
 
     {
         test_device_->flags = DEV_CTX_IMMORTAL | DEV_CTX_MUST_ISOLATE | DEV_CTX_MULTI_BIND;
-        test_device_->set_protocol_id(ZX_PROTOCOL_TEST_PARENT);
         test_device_->name = fbl::String("test", &ac);
         if (!ac.check()) {
             return ZX_ERR_NO_MEMORY;
diff --git a/zircon/system/core/devmgr/devcoordinator/device.cpp b/zircon/system/core/devmgr/devcoordinator/device.cpp
index f88a60540ed..ffd62ee5335 100644
--- a/zircon/system/core/devmgr/devcoordinator/device.cpp
+++ b/zircon/system/core/devmgr/devcoordinator/device.cpp
@@ -14,8 +14,8 @@
 
 namespace devmgr {
 
-Device::Device(Coordinator* coord, fbl::RefPtr<Device> parent)
-    : coordinator(coord), parent_(std::move(parent)),
+Device::Device(Coordinator* coord, fbl::RefPtr<Device> parent, uint32_t protocol_id)
+    : coordinator(coord), parent_(std::move(parent)), protocol_id_(protocol_id),
       publish_task_([this] { coordinator->HandleNewDevice(fbl::WrapRefPtr(this)); }) {}
 
 Device::~Device() {
@@ -60,7 +60,7 @@ zx_status_t Device::Create(Coordinator* coordinator, const fbl::RefPtr<Device>&
         real_parent = parent;
     }
 
-    auto dev = fbl::MakeRefCounted<Device>(coordinator, real_parent);
+    auto dev = fbl::MakeRefCounted<Device>(coordinator, real_parent, protocol_id);
     if (!dev) {
         return ZX_ERR_NO_MEMORY;
     }
@@ -73,7 +73,6 @@ zx_status_t Device::Create(Coordinator* coordinator, const fbl::RefPtr<Device>&
     dev->libname = std::move(driver_path);
     dev->args = std::move(args);
     dev->set_channel(std::move(rpc));
-    dev->set_protocol_id(protocol_id);
     dev->client_remote = std::move(client_remote);
 
     // If we have bus device args we are, by definition, a bus device.
@@ -117,7 +116,7 @@ zx_status_t Device::CreateComposite(Coordinator* coordinator, Devhost* devhost,
                                        composite_props.size());
     memcpy(props.get(), composite_props.get(), props.size() * sizeof(props[0]));
 
-    auto dev = fbl::MakeRefCounted<Device>(coordinator, nullptr);
+    auto dev = fbl::MakeRefCounted<Device>(coordinator, nullptr, ZX_PROTOCOL_COMPOSITE);
     if (!dev) {
         return ZX_ERR_NO_MEMORY;
     }
@@ -129,7 +128,6 @@ zx_status_t Device::CreateComposite(Coordinator* coordinator, Devhost* devhost,
 
     dev->name = composite.name();
     dev->set_channel(std::move(rpc));
-    dev->set_protocol_id(ZX_PROTOCOL_COMPOSITE);
     // We exist within our parent's device host
     dev->set_host(devhost);
 
@@ -157,7 +155,7 @@ zx_status_t Device::CreateComposite(Coordinator* coordinator, Devhost* devhost,
 zx_status_t Device::CreateProxy() {
     ZX_ASSERT(this->proxy == nullptr);
 
-    auto dev = fbl::MakeRefCounted<Device>(this->coordinator, fbl::WrapRefPtr(this));
+    auto dev = fbl::MakeRefCounted<Device>(this->coordinator, fbl::WrapRefPtr(this), protocol_id_);
     if (dev == nullptr) {
         return ZX_ERR_NO_MEMORY;
     }
@@ -177,7 +175,6 @@ zx_status_t Device::CreateProxy() {
     }
 
     dev->flags = DEV_CTX_PROXY;
-    dev->set_protocol_id(protocol_id_);
     this->proxy = std::move(dev);
     log(DEVLC, "devcoord: dev %p name='%s' (proxy)\n", this, this->name.data());
     return ZX_OK;
diff --git a/zircon/system/core/devmgr/devcoordinator/device.h b/zircon/system/core/devmgr/devcoordinator/device.h
index c12d77cdfdf..9caa7628bc8 100644
--- a/zircon/system/core/devmgr/devcoordinator/device.h
+++ b/zircon/system/core/devmgr/devcoordinator/device.h
@@ -56,7 +56,7 @@ class SuspendContext;
 // clang-format on
 
 struct Device : public fbl::RefCounted<Device>, public AsyncLoopRefCountedRpcHandler<Device> {
-    Device(Coordinator* coord, fbl::RefPtr<Device> parent);
+    Device(Coordinator* coord, fbl::RefPtr<Device> parent, uint32_t protocol_id);
     ~Device();
 
     // Create a new device with the given parameters.  This sets up its
@@ -208,9 +208,6 @@ struct Device : public fbl::RefCounted<Device>, public AsyncLoopRefCountedRpcHan
     fbl::RefPtr<const Device> parent() const { return parent_; }
 
     uint32_t protocol_id() const { return protocol_id_; }
-    // TODO: Remove set_protocol_id once this class is further encapsulated.  It
-    // should be unnecessary.
-    void set_protocol_id(uint32_t protocol_id) { protocol_id_ = protocol_id; }
 
     bool is_bindable() const {
         return !(flags & (DEV_CTX_BOUND | DEV_CTX_DEAD | DEV_CTX_INVISIBLE));
@@ -259,7 +256,7 @@ private:
     zx_status_t HandleRead();
 
     fbl::RefPtr<Device> parent_;
-    uint32_t protocol_id_ = 0;
+    const uint32_t protocol_id_;
 
     fbl::Array<const zx_device_prop_t> props_;
     // If the device has a topological property in |props|, this points to it.
-- 
GitLab