From 6e38b53001603145ec4e52a6c07fabc300e3ace2 Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Fri, 16 Aug 2019 11:42:27 +0200 Subject: [PATCH] libc: use Id_space for FD allocator This patch replaces the former use of an Allocator_avl with the Id_space utility, which is safer to use and allows for the iteration of all elements. The iteration over open file descriptors is needed for implementing 'fork'. Issue #3478 --- repos/libports/include/libc-plugin/fd_alloc.h | 44 ++++++++++----- repos/libports/src/lib/libc/fd_alloc.cc | 53 +++++++++---------- 2 files changed, 56 insertions(+), 41 deletions(-) diff --git a/repos/libports/include/libc-plugin/fd_alloc.h b/repos/libports/include/libc-plugin/fd_alloc.h index 79009ef2f..3f955e5a1 100644 --- a/repos/libports/include/libc-plugin/fd_alloc.h +++ b/repos/libports/include/libc-plugin/fd_alloc.h @@ -15,10 +15,11 @@ #ifndef _LIBC_PLUGIN__FD_ALLOC_H_ #define _LIBC_PLUGIN__FD_ALLOC_H_ -#include #include #include #include +#include +#include /* libc includes */ #include @@ -39,13 +40,27 @@ namespace Libc { struct File_descriptor { - int libc_fd = -1; - char const *fd_path = 0; /* for 'fchdir', 'fstat' */ - Plugin *plugin = 0; - Plugin_context *context = 0; - int flags = 0; /* for 'fcntl' */ - bool cloexec = 0; /* for 'fcntl' */ - Genode::Lock lock; + Genode::Lock lock { }; + + typedef Genode::Id_space Id_space; + Id_space::Element _elem; + + int const libc_fd = _elem.id().value; + + char const *fd_path = nullptr; /* for 'fchdir', 'fstat' */ + + Plugin *plugin; + Plugin_context *context; + + int flags = 0; /* for 'fcntl' */ + bool cloexec = 0; /* for 'fcntl' */ + + File_descriptor(Id_space &id_space, Plugin &plugin, Plugin_context &context) + : _elem(*this, id_space), plugin(&plugin), context(&context) { } + + File_descriptor(Id_space &id_space, Plugin &plugin, Plugin_context &context, + Id_space::Id id) + : _elem(*this, id_space, id), plugin(&plugin), context(&context) { } void path(char const *newpath) { @@ -54,8 +69,7 @@ namespace Libc { Genode::size_t const path_size = ::strlen(newpath) + 1; char *buf = (char*)malloc(path_size); if (!buf) { - Genode::error("could not allocate path buffer for libc_fd ", - libc_fd, libc_fd == ANY_FD ? " (any)" : ""); + Genode::error("could not allocate path buffer for libc_fd ", libc_fd); return; } ::memcpy(buf, newpath, path_size); @@ -66,18 +80,24 @@ namespace Libc { }; - class File_descriptor_allocator : Allocator_avl_tpl + class File_descriptor_allocator { private: Genode::Lock _lock; + Genode::Allocator &_alloc; + + typedef File_descriptor::Id_space Id_space; + + Id_space _id_space; + public: /** * Constructor */ - File_descriptor_allocator(Genode::Allocator &md_alloc); + File_descriptor_allocator(Genode::Allocator &_alloc); /** * Allocate file descriptor diff --git a/repos/libports/src/lib/libc/fd_alloc.cc b/repos/libports/src/lib/libc/fd_alloc.cc index b53a44331..db27f5c75 100644 --- a/repos/libports/src/lib/libc/fd_alloc.cc +++ b/repos/libports/src/lib/libc/fd_alloc.cc @@ -40,11 +40,9 @@ File_descriptor_allocator *Libc::file_descriptor_allocator() } -File_descriptor_allocator::File_descriptor_allocator(Genode::Allocator &md_alloc) -: Allocator_avl_tpl(&md_alloc) -{ - add_range(0, MAX_NUM_FDS); -} +File_descriptor_allocator::File_descriptor_allocator(Genode::Allocator &alloc) +: _alloc(alloc) +{ } File_descriptor *File_descriptor_allocator::alloc(Plugin *plugin, @@ -53,44 +51,41 @@ File_descriptor *File_descriptor_allocator::alloc(Plugin *plugin, { Lock::Guard guard(_lock); - /* we use addresses returned by the allocator as file descriptors */ - addr_t addr = (libc_fd <= ANY_FD ? ANY_FD : libc_fd); + bool const any_fd = (libc_fd < 0); + if (any_fd) + return new (_alloc) File_descriptor(_id_space, *plugin, *context); - /* allocate fresh fd if the default value for 'libc_fd' was specified */ - bool alloc_ok = false; - if (libc_fd <= ANY_FD) - alloc_ok = Allocator_avl_base::alloc_aligned(1, reinterpret_cast(&addr), 0).ok(); - else - alloc_ok = (Allocator_avl_base::alloc_addr(1, addr).ok()); - - if (!alloc_ok) { - error("could not allocate libc_fd ", libc_fd, - libc_fd <= ANY_FD ? " (any)" : ""); - return 0; - } - - File_descriptor *fdo = metadata((void*)addr); - fdo->libc_fd = (int)addr; - fdo->fd_path = 0; - fdo->plugin = plugin; - fdo->context = context; - fdo->lock = Lock(Lock::UNLOCKED); - return fdo; + Id_space::Id const id {(unsigned)libc_fd}; + return new (_alloc) File_descriptor(_id_space, *plugin, *context, id); } void File_descriptor_allocator::free(File_descriptor *fdo) { Lock::Guard guard(_lock); + ::free((void *)fdo->fd_path); - Allocator_avl_base::free(reinterpret_cast(fdo->libc_fd)); + + Genode::destroy(_alloc, fdo); } File_descriptor *File_descriptor_allocator::find_by_libc_fd(int libc_fd) { Lock::Guard guard(_lock); - return metadata(reinterpret_cast(libc_fd)); + + if (libc_fd < 0) + return nullptr; + + File_descriptor *result = nullptr; + + try { + Id_space::Id const id {(unsigned)libc_fd}; + _id_space.apply(id, [&] (File_descriptor &fd) { + result = &fd; }); + } catch (Id_space::Unknown_id) { } + + return result; }