Skip to content

Commit

Permalink
[fdf][compat] Add parent to Device
Browse files Browse the repository at this point in the history
Add a parent field to each Device. This optional field
will be std::nullopt when the Device is created by
the Driver object in the DFv1 shim.

Bug: 90629

Change-Id: Ie5a073524999109c8b3a5aa48d1fd8658f00202d
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/628462
Reviewed-by: Suraj Malhotra <surajmalhotra@google.com>
Reviewed-by: Abdulla Kamar <abdulla@google.com>
Commit-Queue: David Gilhooley <dgilhooley@google.com>
  • Loading branch information
gilhooleyd authored and Commit Bot committed Jan 11, 2022
1 parent 75e2879 commit c346ef1
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 16 deletions.
9 changes: 5 additions & 4 deletions src/devices/misc/drivers/compat/device.cc
Expand Up @@ -41,13 +41,14 @@ bool HasOp(const zx_protocol_device_t* ops, T member) {
namespace compat {

Device::Device(std::string_view name, void* context, const zx_protocol_device_t* ops,
std::optional<Device*> linked_device, driver::Logger& logger,
async_dispatcher_t* dispatcher)
std::optional<Device*> parent, std::optional<Device*> linked_device,
driver::Logger& logger, async_dispatcher_t* dispatcher)
: name_(name),
context_(context),
ops_(ops),
logger_(logger),
dispatcher_(dispatcher),
parent_(parent),
linked_device_(linked_device ? **linked_device : *this) {}

zx_device_t* Device::ZxDevice() { return static_cast<zx_device_t*>(this); }
Expand All @@ -68,8 +69,8 @@ const char* Device::Name() const { return name_.data(); }
bool Device::HasChildren() const { return child_counter_.use_count() > 1; }

zx_status_t Device::Add(device_add_args_t* zx_args, zx_device_t** out) {
auto device = std::make_unique<Device>(zx_args->name, zx_args->ctx, zx_args->ops, std::nullopt,
logger_, dispatcher_);
auto device = std::make_unique<Device>(zx_args->name, zx_args->ctx, zx_args->ops, this,
std::nullopt, logger_, dispatcher_);
auto device_ptr = device.get();

// Create NodeAddArgs from `zx_args`.
Expand Down
14 changes: 12 additions & 2 deletions src/devices/misc/drivers/compat/device.h
Expand Up @@ -21,8 +21,8 @@ namespace compat {
class Device {
public:
Device(std::string_view name, void* context, const zx_protocol_device_t* ops,
std::optional<Device*> linked_device, driver::Logger& logger,
async_dispatcher_t* dispatcher);
std::optional<Device*> parent, std::optional<Device*> linked_device,
driver::Logger& logger, async_dispatcher_t* dispatcher);

zx_device_t* ZxDevice();

Expand Down Expand Up @@ -54,6 +54,16 @@ class Device {
driver::Logger& logger_;
async_dispatcher_t* const dispatcher_;

// The device's parent. If this field is set then the Device ptr is guaranteed
// to be non-null. The parent is also guaranteed to outlive its child.
//
// This is used by a Device to free itself, by calling parent_.RemoveChild(this).
//
// parent_ will be std::nullopt when the Device is the fake device created
// by the Driver class in the DFv1 shim. When parent_ is std::nullopt, the
// Device will be freed when the Driver is freed.
std::optional<Device*> parent_;

// Used to link two instances of the same device together.
// If the device is not linked with anything, this will point to `this`.
Device& linked_device_;
Expand Down
26 changes: 17 additions & 9 deletions src/devices/misc/drivers/compat/device_test.cc
Expand Up @@ -71,7 +71,8 @@ TEST_F(DeviceTest, ConstructDevice) {

// Create a device.
zx_protocol_device_t ops{};
compat::Device device("test-device", nullptr, &ops, {}, logger(), dispatcher());
compat::Device device("test-device", nullptr, &ops, std::nullopt, std::nullopt, logger(),
dispatcher());
device.Bind({std::move(endpoints->client), dispatcher()});

// Test basic functions on the device.
Expand Down Expand Up @@ -99,7 +100,8 @@ TEST_F(DeviceTest, AddChildDevice) {

// Create a device.
zx_protocol_device_t ops{};
compat::Device parent("parent", nullptr, &ops, {}, logger(), dispatcher());
compat::Device parent("parent", nullptr, &ops, std::nullopt, std::nullopt, logger(),
dispatcher());
parent.Bind({std::move(endpoints->client), dispatcher()});

// Add a child device.
Expand All @@ -124,7 +126,8 @@ TEST_F(DeviceTest, AddChildDeviceWithInit) {

// Create a device.
zx_protocol_device_t parent_ops{};
compat::Device parent("parent", nullptr, &parent_ops, {}, logger(), dispatcher());
compat::Device parent("parent", nullptr, &parent_ops, std::nullopt, std::nullopt, logger(),
dispatcher());
parent.Bind({std::move(endpoints->client), dispatcher()});

// Add a child device.
Expand Down Expand Up @@ -159,7 +162,8 @@ TEST_F(DeviceTest, AddAndRemoveChildDevice) {

// Create a device.
zx_protocol_device_t ops{};
compat::Device parent("parent", nullptr, &ops, {}, logger(), dispatcher());
compat::Device parent("parent", nullptr, &ops, std::nullopt, std::nullopt, logger(),
dispatcher());
parent.Bind({std::move(endpoints->client), dispatcher()});

// Add a child device.
Expand All @@ -186,22 +190,25 @@ TEST_F(DeviceTest, AddAndRemoveChildDevice) {
TEST_F(DeviceTest, GetProtocolFromDevice) {
// Create a device without a get_protocol hook.
zx_protocol_device_t ops{};
compat::Device without("without-protocol", nullptr, &ops, {}, logger(), dispatcher());
compat::Device without("without-protocol", nullptr, &ops, std::nullopt, std::nullopt, logger(),
dispatcher());
ASSERT_EQ(ZX_ERR_UNAVAILABLE, without.GetProtocol(ZX_PROTOCOL_BLOCK, nullptr));

// Create a device with a get_protocol hook.
ops.get_protocol = [](void* ctx, uint32_t proto_id, void* protocol) {
EXPECT_EQ(ZX_PROTOCOL_BLOCK, proto_id);
return ZX_OK;
};
compat::Device with("with-protocol", nullptr, &ops, {}, logger(), dispatcher());
compat::Device with("with-protocol", nullptr, &ops, std::nullopt, std::nullopt, logger(),
dispatcher());
ASSERT_EQ(ZX_OK, with.GetProtocol(ZX_PROTOCOL_BLOCK, nullptr));
}

TEST_F(DeviceTest, DeviceMetadata) {
// Create a device.
zx_protocol_device_t ops{};
compat::Device device("test-device", nullptr, &ops, {}, logger(), dispatcher());
compat::Device device("test-device", nullptr, &ops, std::nullopt, std::nullopt, logger(),
dispatcher());

// Add metadata to the device.
const uint64_t metadata = 0xAABBCCDDEEFF0011;
Expand Down Expand Up @@ -238,8 +245,9 @@ TEST_F(DeviceTest, DeviceMetadata) {
TEST_F(DeviceTest, LinkedDeviceMetadata) {
// Create two devices.
zx_protocol_device_t ops{};
compat::Device parent("test-parent", nullptr, &ops, {}, logger(), dispatcher());
compat::Device child("test-device", nullptr, &ops, &parent, logger(), dispatcher());
compat::Device parent("test-parent", nullptr, &ops, std::nullopt, std::nullopt, logger(),
dispatcher());
compat::Device child("test-device", nullptr, &ops, std::nullopt, &parent, logger(), dispatcher());

// Add metadata to the parent device.
const uint64_t metadata = 0xAABBCCDDEEFF0011;
Expand Down
2 changes: 1 addition & 1 deletion src/devices/misc/drivers/compat/driver.cc
Expand Up @@ -58,7 +58,7 @@ Driver::Driver(async_dispatcher_t* dispatcher, fidl::WireSharedClient<fdf::Node>
ns_(std::move(ns)),
logger_(std::move(logger)),
url_(url),
device_(name, context, ops, linked_device, inner_logger_, dispatcher) {
device_(name, context, ops, std::nullopt, linked_device, inner_logger_, dispatcher) {
device_.Bind(std::move(node));
}

Expand Down

0 comments on commit c346ef1

Please sign in to comment.