From fef8cdce64149e60d80580cee9f2eef2549fe911 Mon Sep 17 00:00:00 2001
From: Todd Eisenberger <teisenbe@google.com>
Date: Fri, 3 May 2019 21:30:24 +0000
Subject: [PATCH] [devcoordinator] Make topo paths match devfs paths for
 composite devices

Without this, the devfs path would include the composite bindpoint and
the topological path would not.  The topological path has been updated
to include it.

Change-Id: I8185ae93697bab90361eb6a29ba5c37cea82a1cc
---
 .../devcoordinator/coordinator-test.cpp       | 36 +++++++++++++++++++
 .../devmgr/devcoordinator/coordinator.cpp     | 12 +++++--
 .../core/devmgr/devcoordinator/devfs.cpp      | 18 ++++++++++
 .../system/core/devmgr/devcoordinator/devfs.h |  6 ++++
 4 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/zircon/system/core/devmgr/devcoordinator/coordinator-test.cpp b/zircon/system/core/devmgr/devcoordinator/coordinator-test.cpp
index 244c86f2ce9..7e0869fd365 100644
--- a/zircon/system/core/devmgr/devcoordinator/coordinator-test.cpp
+++ b/zircon/system/core/devmgr/devcoordinator/coordinator-test.cpp
@@ -902,6 +902,42 @@ TEST_F(CompositeTestCase, DevfsNotifications) {
     ASSERT_BYTES_EQ(reinterpret_cast<const uint8_t*>(kCompositeDevName), msg + 2, msg[1]);
 }
 
+// Make sure the path returned by GetTopologicalPath is accurate
+TEST_F(CompositeTestCase, Topology) {
+    size_t device_indexes[2];
+    uint32_t protocol_id[] = {
+        ZX_PROTOCOL_GPIO,
+        ZX_PROTOCOL_I2C,
+    };
+    static_assert(fbl::count_of(protocol_id) == fbl::count_of(device_indexes));
+
+    const char* kCompositeDevName = "composite-dev";
+    ASSERT_NO_FATAL_FAILURES(BindCompositeDefineComposite(
+            platform_bus(), protocol_id, fbl::count_of(protocol_id), nullptr /* props */,
+            0, kCompositeDevName));
+    // Add the devices to construct the composite out of.
+    for (size_t i = 0; i < fbl::count_of(device_indexes); ++i) {
+        char name[32];
+        snprintf(name, sizeof(name), "device-%zu", i);
+        ASSERT_NO_FATAL_FAILURES(AddDevice(platform_bus(), name, protocol_id[i], "",
+                                           &device_indexes[i]));
+    }
+
+    zx::channel composite_remote;
+    size_t component_device_indexes[fbl::count_of(device_indexes)];
+    ASSERT_NO_FATAL_FAILURES(CheckCompositeCreation(kCompositeDevName,
+                                                    device_indexes, fbl::count_of(device_indexes),
+                                                    component_device_indexes, &composite_remote));
+
+    devmgr::Devnode* dn = coordinator()->root_device()->self;
+    fbl::RefPtr<devmgr::Device> composite_dev;
+    ASSERT_OK(devmgr::devfs_walk(dn, "composite-dev", &composite_dev));
+
+    char path_buf[PATH_MAX];
+    ASSERT_OK(coordinator()->GetTopologicalPath(composite_dev, path_buf, sizeof(path_buf)));
+    ASSERT_STR_EQ(path_buf, "/dev/composite-dev");
+}
+
 } // namespace
 
 int main(int argc, char** argv) {
diff --git a/zircon/system/core/devmgr/devcoordinator/coordinator.cpp b/zircon/system/core/devmgr/devcoordinator/coordinator.cpp
index 06c8d64575c..5b9cc7904ba 100644
--- a/zircon/system/core/devmgr/devcoordinator/coordinator.cpp
+++ b/zircon/system/core/devmgr/devcoordinator/coordinator.cpp
@@ -345,6 +345,7 @@ zx_status_t Coordinator::GetTopologicalPath(const fbl::RefPtr<const Device>& dev
                                             size_t max) const {
     // TODO: Remove VLA.
     char tmp[max];
+    char name_buf[fuchsia_io_MAX_FILENAME + strlen("dev/")];
     char* path = tmp + max - 1;
     *path = 0;
     size_t total = 1;
@@ -356,10 +357,15 @@ zx_status_t Coordinator::GetTopologicalPath(const fbl::RefPtr<const Device>& dev
         }
 
         const char* name;
-        if (itr->parent()) {
-            name = itr->name().data();
-        } else {
+        if (&*itr == root_device_.get()) {
             name = "dev";
+        } else if (itr->composite() != nullptr) {
+            strcpy(name_buf, "dev/");
+            strncpy(name_buf + strlen("dev/"), itr->name().data(), fuchsia_io_MAX_FILENAME);
+            name_buf[sizeof(name_buf)-1] = 0;
+            name = name_buf;
+        } else {
+            name = itr->name().data();
         }
 
         size_t len = strlen(name) + 1;
diff --git a/zircon/system/core/devmgr/devcoordinator/devfs.cpp b/zircon/system/core/devmgr/devcoordinator/devfs.cpp
index 36c83d21fcc..6981dc426b6 100644
--- a/zircon/system/core/devmgr/devcoordinator/devfs.cpp
+++ b/zircon/system/core/devmgr/devcoordinator/devfs.cpp
@@ -875,4 +875,22 @@ void devfs_init(const fbl::RefPtr<Device>& device, async_dispatcher_t* dispatche
     __UNUSED auto ptr = root_devnode.release();
 }
 
+zx_status_t devfs_walk(Devnode* dn, const char* path, fbl::RefPtr<Device>* dev) {
+    Devnode* inout = dn;
+    char* remainder = nullptr;
+
+    char path_copy[PATH_MAX];
+    if (strlen(path) + 1 > sizeof(path_copy)) {
+        return ZX_ERR_BUFFER_TOO_SMALL;
+    }
+    strcpy(path_copy, path);
+
+    zx_status_t status = devfs_walk(&inout, path_copy, &remainder);
+    if (status != ZX_OK) {
+        return status;
+    }
+    *dev = fbl::WrapRefPtr(inout->device);
+    return ZX_OK;
+}
+
 } // namespace devmgr
diff --git a/zircon/system/core/devmgr/devcoordinator/devfs.h b/zircon/system/core/devmgr/devcoordinator/devfs.h
index 49cb4d614e0..292bf68eb03 100644
--- a/zircon/system/core/devmgr/devcoordinator/devfs.h
+++ b/zircon/system/core/devmgr/devcoordinator/devfs.h
@@ -32,4 +32,10 @@ void devfs_advertise(const fbl::RefPtr<Device>& dev);
 void devfs_advertise_modified(const fbl::RefPtr<Device>& dev);
 zx_status_t devfs_connect(const Device* dev, zx::channel client_remote);
 
+// This method is exposed for testing.  It walks the devfs from the given node,
+// traversing the given sub-path.
+// If ZX_OK is returned, then *device_out refers to the device at the given path
+// relative to the devnode.
+zx_status_t devfs_walk(Devnode* dn, const char* path, fbl::RefPtr<Device>* device_out);
+
 } // namespace devmgr
-- 
GitLab