From 4c4962b30625096432d7eb293371fbe44c34ea97 Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Tue, 18 Jul 2023 13:04:43 +0200 Subject: [PATCH] nic_router: clean-up session creation with class Introduces a new class that does the clean-up if some exception is thrown while creating the session. This reduces redundancy and overall lines of code. Ref #4966 --- .../src/server/nic_router/nic_session_root.cc | 85 ++++---------- .../src/server/nic_router/session_creation.h | 93 +++++++++++++++ .../server/nic_router/uplink_session_root.cc | 106 +++++------------- 3 files changed, 147 insertions(+), 137 deletions(-) create mode 100644 repos/os/src/server/nic_router/session_creation.h diff --git a/repos/os/src/server/nic_router/nic_session_root.cc b/repos/os/src/server/nic_router/nic_session_root.cc index 1b5e98019d..e6dfbfca6d 100644 --- a/repos/os/src/server/nic_router/nic_session_root.cc +++ b/repos/os/src/server/nic_router/nic_session_root.cc @@ -16,6 +16,7 @@ /* local includes */ #include +#include #include using namespace Net; @@ -304,79 +305,39 @@ Net::Nic_session_root::Nic_session_root(Env &env, Nic_session_component *Net::Nic_session_root::_create_session(char const *args) { + Session_creation session_creation { }; try { - /* create session environment temporarily on the stack */ - Session_env session_env_stack { _env, _shared_quota, - Ram_quota { Arg_string::find_arg(args, "ram_quota").ulong_value(0) }, - Cap_quota { Arg_string::find_arg(args, "cap_quota").ulong_value(0) } }; - - /* alloc/attach RAM block and move session env to base of the block */ - Ram_dataspace_capability ram_ds { - session_env_stack.alloc(sizeof(Session_env) + - sizeof(Nic_session_component), CACHED) }; - try { - void * const ram_ptr { session_env_stack.attach(ram_ds) }; - Session_env &session_env { - *construct_at(ram_ptr, session_env_stack) }; - - /* create new session object behind session env in the RAM block */ - try { + return session_creation.execute( + _env, _shared_quota, args, + [&] (Session_env &session_env, void *session_at, Ram_dataspace_capability ram_ds) + { Session_label const label { label_from_args(args) }; - Mac_address const mac { _mac_alloc.alloc() }; + Mac_address const mac { _mac_alloc.alloc() }; try { return construct_at( - (void*)((addr_t)ram_ptr + sizeof(Session_env)), - session_env, + session_at, session_env, Arg_string::find_arg(args, "tx_buf_size").ulong_value(0), Arg_string::find_arg(args, "rx_buf_size").ulong_value(0), _timer, mac, _router_mac, label, _interfaces, _config(), ram_ds); } - catch (Out_of_ram) { + catch (...) { _mac_alloc.free(mac); - Session_env session_env_stack { session_env }; - session_env_stack.detach(ram_ptr); - session_env_stack.free(ram_ds); - _invalid_downlink("NIC session RAM quota"); - throw Insufficient_ram_quota(); + throw; } - catch (Out_of_caps) { - _mac_alloc.free(mac); - Session_env session_env_stack { session_env }; - session_env_stack.detach(ram_ptr); - session_env_stack.free(ram_ds); - _invalid_downlink("NIC session CAP quota"); - throw Insufficient_cap_quota(); - } - } - catch (Mac_allocator::Alloc_failed) { - Session_env session_env_stack { session_env }; - session_env_stack.detach(ram_ptr); - session_env_stack.free(ram_ds); - _invalid_downlink("failed to allocate MAC address"); - throw Service_denied(); - } - } - catch (Region_map::Invalid_dataspace) { - session_env_stack.free(ram_ds); - _invalid_downlink("Failed to attach RAM"); - throw Service_denied(); - } - catch (Region_map::Region_conflict) { - session_env_stack.free(ram_ds); - _invalid_downlink("Failed to attach RAM"); - throw Service_denied(); - } - catch (Out_of_ram) { - session_env_stack.free(ram_ds); - _invalid_downlink("NIC session RAM quota"); - throw Insufficient_ram_quota(); - } - catch (Out_of_caps) { - session_env_stack.free(ram_ds); - _invalid_downlink("NIC session CAP quota"); - throw Insufficient_cap_quota(); - } + }); + } + catch (Mac_allocator::Alloc_failed) { + _invalid_downlink("failed to allocate MAC address"); + throw Service_denied(); + } + catch (Region_map::Invalid_dataspace) { + _invalid_downlink("Failed to attach RAM"); + throw Service_denied(); + } + catch (Region_map::Region_conflict) { + _invalid_downlink("Failed to attach RAM"); + throw Service_denied(); } catch (Out_of_ram) { _invalid_downlink("NIC session RAM quota"); diff --git a/repos/os/src/server/nic_router/session_creation.h b/repos/os/src/server/nic_router/session_creation.h new file mode 100644 index 0000000000..296a854ed6 --- /dev/null +++ b/repos/os/src/server/nic_router/session_creation.h @@ -0,0 +1,93 @@ +/* + * \brief The generic parts of the process of creating a session component + * \author Martin Stein + * \date 2023-07-18 + */ + +/* + * Copyright (C) 2023 Genode Labs GmbH + * + * This file is part of the Genode OS framework, which is distributed + * under the terms of the GNU Affero General Public License version 3. + */ + +#ifndef _SESSION_CREATION_H_ +#define _SESSION_CREATION_H_ + +/* nic_router includes */ +#include + +namespace Net { + + using namespace Genode; + + template + class Session_creation; +} + + +template +class Net::Session_creation +{ + private: + + Constructible _tmp_session_env { }; + Constructible _ram_ds { }; + void *_ram_ptr { nullptr }; + Session_env *_session_env_ptr { nullptr }; + + public: + + template + SESSION_COMPONENT *execute(Env &env, + Quota &shared_quota, + char const *session_args, + CREATE_SESSION_FN && create_session_fn) + { + /* + * Note that this cannot be done in the constructor of this class + * because it is required that the destructor of this class is called + * on any exception that is thrown by the below code. + */ + + /* create session env as temporary member of this object */ + _tmp_session_env.construct(env, shared_quota, + Ram_quota { Arg_string::find_arg(session_args, "ram_quota").ulong_value(0) }, + Cap_quota { Arg_string::find_arg(session_args, "cap_quota").ulong_value(0) }); + + /* alloc/attach ram ds and move session env to the base of it */ + _ram_ds.construct(_tmp_session_env->alloc(sizeof(Session_env) + sizeof(SESSION_COMPONENT), CACHED)); + _ram_ptr = _tmp_session_env->attach(*_ram_ds); + _session_env_ptr = construct_at(_ram_ptr, *_tmp_session_env); + + /* create session right behind the session env inside the ram ds */ + SESSION_COMPONENT *session { create_session_fn(*_session_env_ptr, (void*)((addr_t)_ram_ptr + sizeof(Session_env)), *_ram_ds) }; + + /* ensure that the ram ds is not dissolved on destruction of this object */ + _tmp_session_env.destruct(); + _session_env_ptr = nullptr; + return session; + } + + ~Session_creation() + { + if (_session_env_ptr) { + + Session_env tmp_session_env { *_session_env_ptr }; + if (_ram_ds.constructed()) + tmp_session_env.free(*_ram_ds); + if (_ram_ptr) + tmp_session_env.detach(_ram_ptr); + + } else if (_tmp_session_env.constructed()) { + + if (_ram_ds.constructed()) + _tmp_session_env->free(*_ram_ds); + if (_ram_ptr) + _tmp_session_env->detach(_ram_ptr); + } + } +}; + + +#endif /* _SESSION_CREATION_H_ */ diff --git a/repos/os/src/server/nic_router/uplink_session_root.cc b/repos/os/src/server/nic_router/uplink_session_root.cc index 28ee10e798..fda142b129 100644 --- a/repos/os/src/server/nic_router/uplink_session_root.cc +++ b/repos/os/src/server/nic_router/uplink_session_root.cc @@ -17,6 +17,7 @@ /* local includes */ #include #include +#include using namespace Net; using namespace Genode; @@ -136,88 +137,43 @@ Net::Uplink_session_root::Uplink_session_root(Env &env, Uplink_session_component * Net::Uplink_session_root::_create_session(char const *args) { + Session_creation session_creation { }; try { - /* create session environment temporarily on the stack */ - Session_env session_env_stack { _env, _shared_quota, - Ram_quota { Arg_string::find_arg(args, "ram_quota").ulong_value(0) }, - Cap_quota { Arg_string::find_arg(args, "cap_quota").ulong_value(0) } }; + return session_creation.execute( + _env, _shared_quota, args, + [&] (Session_env &session_env, void *session_at, Ram_dataspace_capability ram_ds) + { + Session_label const label { label_from_args(args) }; - /* alloc/attach RAM block and move session env to base of the block */ - Ram_dataspace_capability ram_ds { - session_env_stack.alloc(sizeof(Session_env) + - sizeof(Uplink_session_component), CACHED) }; - try { - void * const ram_ptr { session_env_stack.attach(ram_ds) }; - Session_env &session_env { - *construct_at(ram_ptr, session_env_stack) }; + enum { MAC_STR_LENGTH = 19 }; + char mac_str [MAC_STR_LENGTH]; + Arg mac_arg { Arg_string::find_arg(args, "mac_address") }; - Session_label const label { label_from_args(args) }; - - enum { MAC_STR_LENGTH = 19 }; - char mac_str [MAC_STR_LENGTH]; - Arg mac_arg { Arg_string::find_arg(args, "mac_address") }; - - if (!mac_arg.valid()) { - Session_env session_env_stack { session_env }; - session_env_stack.detach(ram_ptr); - session_env_stack.free(ram_ds); - _invalid_downlink("failed to find 'mac_address' arg"); - throw Service_denied(); - } - mac_arg.string(mac_str, MAC_STR_LENGTH, ""); - Mac_address mac { }; - ascii_to(mac_str, mac); - if (mac == Mac_address { }) { - Session_env session_env_stack { session_env }; - session_env_stack.detach(ram_ptr); - session_env_stack.free(ram_ds); - _invalid_downlink("malformed 'mac_address' arg"); - throw Service_denied(); - } - /* create new session object behind session env in the RAM block */ - try { + if (!mac_arg.valid()) { + _invalid_downlink("failed to find 'mac_address' arg"); + throw Service_denied(); + } + mac_arg.string(mac_str, MAC_STR_LENGTH, ""); + Mac_address mac { }; + ascii_to(mac_str, mac); + if (mac == Mac_address { }) { + _invalid_downlink("malformed 'mac_address' arg"); + throw Service_denied(); + } return construct_at( - (void*)((addr_t)ram_ptr + sizeof(Session_env)), - session_env, + session_at, session_env, Arg_string::find_arg(args, "tx_buf_size").ulong_value(0), Arg_string::find_arg(args, "rx_buf_size").ulong_value(0), _timer, mac, label, _interfaces, _config(), ram_ds); - } - catch (Out_of_ram) { - Session_env session_env_stack { session_env }; - session_env_stack.detach(ram_ptr); - session_env_stack.free(ram_ds); - _invalid_downlink("Uplink session RAM quota"); - throw Insufficient_ram_quota(); - } - catch (Out_of_caps) { - Session_env session_env_stack { session_env }; - session_env_stack.detach(ram_ptr); - session_env_stack.free(ram_ds); - _invalid_downlink("Uplink session CAP quota"); - throw Insufficient_cap_quota(); - } - } - catch (Region_map::Invalid_dataspace) { - session_env_stack.free(ram_ds); - _invalid_downlink("Failed to attach RAM"); - throw Service_denied(); - } - catch (Region_map::Region_conflict) { - session_env_stack.free(ram_ds); - _invalid_downlink("Failed to attach RAM"); - throw Service_denied(); - } - catch (Out_of_ram) { - session_env_stack.free(ram_ds); - _invalid_downlink("Uplink session RAM quota"); - throw Insufficient_ram_quota(); - } - catch (Out_of_caps) { - session_env_stack.free(ram_ds); - _invalid_downlink("Uplink session CAP quota"); - throw Insufficient_cap_quota(); - } + }); + } + catch (Region_map::Invalid_dataspace) { + _invalid_downlink("Failed to attach RAM"); + throw Service_denied(); + } + catch (Region_map::Region_conflict) { + _invalid_downlink("Failed to attach RAM"); + throw Service_denied(); } catch (Out_of_ram) { _invalid_downlink("Uplink session RAM quota");