Kernel: Make Device::after_inserting to return ErrorOr<void>

Instead of just returning nothing, let's return Error or nothing.
This would help later on with error propagation in case of failure
during this method.

This also makes us more paranoid about failure in this method, so when
initializing a DisplayConnector we safely tear down the internal members
of the object. This applies the same for a StorageDevice object, but its
after_inserting method is much smaller compared to the DisplayConnector
overriden method.
This commit is contained in:
Liav A 2022-12-21 22:33:56 +02:00 committed by Andrew Kaster
parent 15c0efede9
commit 25bb293629
Notes: sideshowbarker 2024-07-17 05:23:40 +09:00
9 changed files with 61 additions and 26 deletions

View file

@ -32,13 +32,14 @@ void Device::after_inserting_add_to_device_management()
DeviceManagement::the().after_inserting_device({}, *this);
}
void Device::after_inserting()
ErrorOr<void> Device::after_inserting()
{
after_inserting_add_to_device_management();
VERIFY(!m_sysfs_component);
auto sys_fs_component = SysFSDeviceComponent::must_create(*this);
m_sysfs_component = sys_fs_component;
after_inserting_add_to_device_identifier_directory();
return {};
}
void Device::will_be_destroyed()

View file

@ -50,7 +50,7 @@ public:
virtual bool is_device() const override { return true; }
virtual void will_be_destroyed() override;
virtual void after_inserting();
virtual ErrorOr<void> after_inserting();
virtual bool is_openable_by_jailed_processes() const { return false; }
void process_next_queued_request(Badge<AsyncDeviceRequest>, AsyncDeviceRequest const&);

View file

@ -57,7 +57,7 @@ public:
requires(requires(Args... args) { DeviceType::try_create(args...); })
{
auto device = TRY(DeviceType::try_create(forward<Args>(args)...));
device->after_inserting();
TRY(device->after_inserting());
return device;
}
@ -65,7 +65,7 @@ public:
static inline ErrorOr<NonnullLockRefPtr<DeviceType>> try_create_device(Args&&... args)
{
auto device = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) DeviceType(forward<Args>(args)...)));
device->after_inserting();
TRY(device->after_inserting());
return device;
}

View file

@ -55,43 +55,73 @@ void DisplayConnector::will_be_destroyed()
{
GraphicsManagement::the().detach_display_connector({}, *this);
VERIFY(m_symlink_sysfs_component);
before_will_be_destroyed_remove_symlink_from_device_identifier_directory();
// NOTE: We check if m_symlink_sysfs_component is not null, because if we failed
// at some point in DisplayConnector::after_inserting(), then that method will tear down
// the object internal members safely, so we don't want to do it again here.
if (m_symlink_sysfs_component) {
before_will_be_destroyed_remove_symlink_from_device_identifier_directory();
m_symlink_sysfs_component.clear();
}
// NOTE: We check if m_sysfs_device_directory is not null, because if we failed
// at some point in DisplayConnector::after_inserting(), then that method will tear down
// the object internal members safely, so we don't want to do it again here.
if (m_sysfs_device_directory) {
SysFSDisplayConnectorsDirectory::the().unplug({}, *m_sysfs_device_directory);
m_sysfs_device_directory.clear();
}
m_symlink_sysfs_component.clear();
SysFSDisplayConnectorsDirectory::the().unplug({}, *m_sysfs_device_directory);
before_will_be_destroyed_remove_from_device_management();
}
void DisplayConnector::after_inserting()
ErrorOr<void> DisplayConnector::after_inserting()
{
after_inserting_add_to_device_management();
ArmedScopeGuard clean_from_device_management([&] {
before_will_be_destroyed_remove_from_device_management();
});
auto sysfs_display_connector_device_directory = DisplayConnectorSysFSDirectory::create(SysFSDisplayConnectorsDirectory::the(), *this);
m_sysfs_device_directory = sysfs_display_connector_device_directory;
SysFSDisplayConnectorsDirectory::the().plug({}, *sysfs_display_connector_device_directory);
ArmedScopeGuard clean_from_sysfs_display_connector_device_directory([&] {
SysFSDisplayConnectorsDirectory::the().unplug({}, *m_sysfs_device_directory);
m_sysfs_device_directory.clear();
});
VERIFY(!m_symlink_sysfs_component);
auto sys_fs_component = MUST(SysFSSymbolicLinkDeviceComponent::try_create(SysFSCharacterDevicesDirectory::the(), *this, *m_sysfs_device_directory));
auto sys_fs_component = TRY(SysFSSymbolicLinkDeviceComponent::try_create(SysFSCharacterDevicesDirectory::the(), *this, *m_sysfs_device_directory));
m_symlink_sysfs_component = sys_fs_component;
after_inserting_add_symlink_to_device_identifier_directory();
ArmedScopeGuard clean_symlink_to_device_identifier_directory([&] {
VERIFY(m_symlink_sysfs_component);
before_will_be_destroyed_remove_symlink_from_device_identifier_directory();
m_symlink_sysfs_component.clear();
});
auto rounded_size = MUST(Memory::page_round_up(m_framebuffer_resource_size));
auto rounded_size = TRY(Memory::page_round_up(m_framebuffer_resource_size));
if (!m_framebuffer_at_arbitrary_physical_range) {
VERIFY(m_framebuffer_address.value().page_base() == m_framebuffer_address.value());
m_shared_framebuffer_vmobject = MUST(Memory::SharedFramebufferVMObject::try_create_for_physical_range(m_framebuffer_address.value(), rounded_size));
m_framebuffer_region = MUST(MM.allocate_kernel_region(m_framebuffer_address.value().page_base(), rounded_size, "Framebuffer"sv, Memory::Region::Access::ReadWrite));
m_shared_framebuffer_vmobject = TRY(Memory::SharedFramebufferVMObject::try_create_for_physical_range(m_framebuffer_address.value(), rounded_size));
m_framebuffer_region = TRY(MM.allocate_kernel_region(m_framebuffer_address.value().page_base(), rounded_size, "Framebuffer"sv, Memory::Region::Access::ReadWrite));
} else {
m_shared_framebuffer_vmobject = MUST(Memory::SharedFramebufferVMObject::try_create_at_arbitrary_physical_range(rounded_size));
m_framebuffer_region = MUST(MM.allocate_kernel_region_with_vmobject(m_shared_framebuffer_vmobject->real_writes_framebuffer_vmobject(), rounded_size, "Framebuffer"sv, Memory::Region::Access::ReadWrite));
m_shared_framebuffer_vmobject = TRY(Memory::SharedFramebufferVMObject::try_create_at_arbitrary_physical_range(rounded_size));
m_framebuffer_region = TRY(MM.allocate_kernel_region_with_vmobject(m_shared_framebuffer_vmobject->real_writes_framebuffer_vmobject(), rounded_size, "Framebuffer"sv, Memory::Region::Access::ReadWrite));
}
m_framebuffer_data = m_framebuffer_region->vaddr().as_ptr();
m_fake_writes_framebuffer_region = MUST(MM.allocate_kernel_region_with_vmobject(m_shared_framebuffer_vmobject->fake_writes_framebuffer_vmobject(), rounded_size, "Fake Writes Framebuffer"sv, Memory::Region::Access::ReadWrite));
m_fake_writes_framebuffer_region = TRY(MM.allocate_kernel_region_with_vmobject(m_shared_framebuffer_vmobject->fake_writes_framebuffer_vmobject(), rounded_size, "Fake Writes Framebuffer"sv, Memory::Region::Access::ReadWrite));
clean_from_device_management.disarm();
clean_from_sysfs_display_connector_device_directory.disarm();
clean_symlink_to_device_identifier_directory.disarm();
GraphicsManagement::the().attach_new_display_connector({}, *this);
if (m_enable_write_combine_optimization) {
[[maybe_unused]] auto result = m_framebuffer_region->set_write_combine(true);
}
return {};
}
bool DisplayConnector::console_mode() const

View file

@ -146,7 +146,7 @@ private:
DisplayConnector(DisplayConnector&&) = delete;
virtual void will_be_destroyed() override;
virtual void after_inserting() override;
virtual ErrorOr<void> after_inserting() override;
ErrorOr<bool> ioctl_requires_ownership(unsigned request) const;

View file

@ -36,23 +36,27 @@ StorageDevice::StorageDevice(Badge<RamdiskDevice>, LUNAddress logical_unit_numbe
{
}
void StorageDevice::after_inserting()
ErrorOr<void> StorageDevice::after_inserting()
{
after_inserting_add_to_device_management();
auto sysfs_storage_device_directory = StorageDeviceSysFSDirectory::create(SysFSStorageDirectory::the(), *this);
m_sysfs_device_directory = sysfs_storage_device_directory;
SysFSStorageDirectory::the().plug({}, *sysfs_storage_device_directory);
VERIFY(!m_symlink_sysfs_component);
auto sys_fs_component = MUST(SysFSSymbolicLinkDeviceComponent::try_create(SysFSBlockDevicesDirectory::the(), *this, *m_sysfs_device_directory));
auto sys_fs_component = TRY(SysFSSymbolicLinkDeviceComponent::try_create(SysFSBlockDevicesDirectory::the(), *this, *m_sysfs_device_directory));
m_symlink_sysfs_component = sys_fs_component;
after_inserting_add_symlink_to_device_identifier_directory();
return {};
}
void StorageDevice::will_be_destroyed()
{
VERIFY(m_symlink_sysfs_component);
before_will_be_destroyed_remove_symlink_from_device_identifier_directory();
m_symlink_sysfs_component.clear();
// NOTE: We check if m_symlink_sysfs_component is not null, because if we failed
// in StorageDevice::after_inserting(), then that method will not set m_symlink_sysfs_component.
if (m_symlink_sysfs_component) {
before_will_be_destroyed_remove_symlink_from_device_identifier_directory();
m_symlink_sysfs_component.clear();
}
SysFSStorageDirectory::the().unplug({}, *m_sysfs_device_directory);
before_will_be_destroyed_remove_from_device_management();
}

View file

@ -88,7 +88,7 @@ protected:
virtual StringView class_name() const override;
private:
virtual void after_inserting() override;
virtual ErrorOr<void> after_inserting() override;
virtual void will_be_destroyed() override;
mutable IntrusiveListNode<StorageDevice, LockRefPtr<StorageDevice>> m_list_node;

View file

@ -22,8 +22,8 @@ ErrorOr<NonnullLockRefPtr<MasterPTY>> MasterPTY::try_create(unsigned int index)
auto master_pty = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) MasterPTY(index, move(buffer))));
auto slave_pty = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) SlavePTY(*master_pty, index)));
master_pty->m_slave = slave_pty;
master_pty->after_inserting();
slave_pty->after_inserting();
TRY(master_pty->after_inserting());
TRY(slave_pty->after_inserting());
return master_pty;
}

View file

@ -35,7 +35,7 @@ UNMAP_AFTER_INIT PTYMultiplexer::~PTYMultiplexer() = default;
UNMAP_AFTER_INIT void PTYMultiplexer::initialize()
{
the().after_inserting();
MUST(the().after_inserting());
}
ErrorOr<NonnullLockRefPtr<OpenFileDescription>> PTYMultiplexer::open(int options)