Skip to content

Commit 1b7f0b2

Browse files
Ian Fishergnoliyil
authored andcommitted
[devfs] Move walk method out of Devnode into test
Devnode::walk was only used in the one test file; the function is brittle (see https://fxbug.dev/112401 for instance) and we would like to avoid accidental future dependencies on it. Change-Id: I5da1b2af748087a7c01ee5df0a7b05f55d92a75f Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/773664 Reviewed-by: David Gilhooley <[email protected]> Commit-Queue: Auto-Submit <[email protected]> Fuchsia-Auto-Submit: Ian Fisher <[email protected]> Reviewed-by: Tamir Duberstein <[email protected]>
1 parent 9fea9e7 commit 1b7f0b2

File tree

3 files changed

+32
-28
lines changed

3 files changed

+32
-28
lines changed

src/devices/bin/driver_manager/devfs/devfs.cc

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -88,30 +88,6 @@ void Devnode::advertise_modified() {
8888
parent_->Notify(name(), fio::wire::WatchEvent::kAdded);
8989
}
9090

91-
zx::result<Devnode*> Devnode::walk(std::string_view path) {
92-
Devnode* dn = this;
93-
94-
while (!path.empty()) {
95-
const size_t i = path.find('/');
96-
if (i == 0) {
97-
return zx::error(ZX_ERR_BAD_PATH);
98-
}
99-
std::string_view name = path;
100-
if (i != std::string::npos) {
101-
name = path.substr(0, i);
102-
path = path.substr(i + 1);
103-
} else {
104-
path = {};
105-
}
106-
fbl::RefPtr<fs::Vnode> out;
107-
if (const zx_status_t status = dn->children().Lookup(name, &out); status != ZX_OK) {
108-
return zx::error(status);
109-
}
110-
dn = &fbl::RefPtr<Devnode::VnodeImpl>::Downcast(out)->holder_;
111-
}
112-
return zx::ok(dn);
113-
}
114-
11591
Devnode::VnodeImpl::VnodeImpl(Devnode& holder, Target target)
11692
: holder_(holder), target_(std::move(target)) {}
11793

src/devices/bin/driver_manager/devfs/devfs.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,6 @@ class Devnode {
5959
Devnode(Devnode&&) = delete;
6060
Devnode& operator=(Devnode&&) = delete;
6161

62-
zx::result<Devnode*> walk(std::string_view path);
63-
6462
// Add a child to this devnode with a given `name`, `protocol`, and `remote.
6563
// The child will be constructed in `out_child`.
6664
zx_status_t add_child(std::string_view name, uint32_t protocol, Remote remote,

src/devices/bin/driver_manager/tests/composite_device_tests.cc

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,36 @@ void CompositeAddOrderTestCase::ExecuteTest(AddLocation add) {
393393
std::size(device_indexes), fragment_device_indexes,
394394
&composite));
395395
}
396+
397+
zx::result<Devnode*> resolve_path(Devnode* dn, std::string_view path) {
398+
// This function does not work with /dev/class entries due to the downcast to Devnode::VnodeImpl.
399+
if (cpp20::starts_with(path, "class/")) {
400+
ADD_FAILURE("resolve_path does not work with /dev/class entry: %.*s",
401+
static_cast<int>(path.size()), path.data());
402+
return zx::error(ZX_ERR_INVALID_ARGS);
403+
}
404+
405+
while (!path.empty()) {
406+
const size_t i = path.find('/');
407+
if (i == 0) {
408+
return zx::error(ZX_ERR_BAD_PATH);
409+
}
410+
std::string_view name = path;
411+
if (i != std::string::npos) {
412+
name = path.substr(0, i);
413+
path = path.substr(i + 1);
414+
} else {
415+
path = {};
416+
}
417+
fbl::RefPtr<fs::Vnode> out;
418+
if (const zx_status_t status = dn->children().Lookup(name, &out); status != ZX_OK) {
419+
return zx::error(status);
420+
}
421+
dn = &fbl::RefPtr<Devnode::VnodeImpl>::Downcast(out)->holder_;
422+
}
423+
return zx::ok(dn);
424+
}
425+
396426
TEST_F(CompositeAddOrderTestCase, DefineBeforeDevices) {
397427
ASSERT_NO_FATAL_FAILURE(ExecuteTest(AddLocation::BEFORE));
398428
}
@@ -933,9 +963,9 @@ TEST_F(CompositeTestCase, Topology) {
933963
ASSERT_OK(path.status_value());
934964

935965
const std::string_view parent_topological_path{path.value().c_str() + kDev.size()};
936-
zx::result parent = root.walk(parent_topological_path);
966+
zx::result parent = resolve_path(&root, parent_topological_path);
937967
ASSERT_OK(parent);
938-
zx::result child = parent.value()->walk(kCompositeDevName);
968+
zx::result child = resolve_path(parent.value(), kCompositeDevName);
939969
if (i != 0) {
940970
// This isn't the primary parent; the composite device isn't a child node.
941971
ASSERT_STATUS(child.status_value(), ZX_ERR_NOT_FOUND);

0 commit comments

Comments
 (0)