Remove unused FunctionDefinition and redundant FunctionsMap (#479)

* remove unused FunctionDefinition

Signed-off-by: Jorge Prendes <[email protected]>

* Remove FunctionsMap

Signed-off-by: Jorge Prendes <[email protected]>

---------

Signed-off-by: Jorge Prendes <[email protected]>

Author: jprendes

src/hyperlight_host/src/func/host_functions.rs

@@ -17,42 +17,14 @@ limitations under the License.
 #![allow(non_snake_case)]
 use std::sync::{Arc, Mutex};
 
-use hyperlight_common::flatbuffer_wrappers::function_types::{
-    ParameterType, ParameterValue, ReturnType,
-};
+use hyperlight_common::flatbuffer_wrappers::function_types::ParameterValue;
 use tracing::{instrument, Span};
 
 use super::{HyperlightFunction, SupportedParameterType, SupportedReturnType};
 use crate::sandbox::{ExtraAllowedSyscall, UninitializedSandbox};
 use crate::HyperlightError::UnexpectedNoOfArguments;
 use crate::{log_then_return, new_error, Result};
 
-/// The definition of a function exposed from the host to the guest
-#[derive(Debug, Default, Clone, PartialEq, Eq)]
-pub struct HostFunctionDefinition {
-    /// The function name
-    pub function_name: String,
-    /// The type of the parameter values for the host function call.
-    pub parameter_types: Option<Vec<ParameterType>>,
-    /// The type of the return value from the host function call
-    pub return_type: ReturnType,
-}
-
-impl HostFunctionDefinition {
-    /// Create a new `HostFunctionDefinition`.
-    pub fn new(
-        function_name: String,
-        parameter_types: Option<Vec<ParameterType>>,
-        return_type: ReturnType,
-    ) -> Self {
-        Self {
-            function_name,
-            parameter_types,
-            return_type,
-        }
-    }
-}
-
 /// Trait for registering a host function
 pub trait HostFunction<R, Args> {
     /// Register the host function with the given name in the sandbox.
@@ -181,8 +153,6 @@ macro_rules! impl_host_function {
                     Ok(result.into_value())
                 });
 
-                let parameter_types = Some(vec![$($P::TYPE),*]);
-
                 if let Some(_eas) = extra_allowed_syscalls {
                     if cfg!(all(feature = "seccomp", target_os = "linux")) {
                         // Register with extra allowed syscalls
@@ -193,11 +163,7 @@ macro_rules! impl_host_function {
                                 .try_lock()
                                 .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?
                                 .register_host_function_with_syscalls(
-                                    &HostFunctionDefinition::new(
-                                        name.to_string(),
-                                        parameter_types,
-                                        R::TYPE,
-                                    ),
+                                    name.to_string(),
                                     HyperlightFunction::new(func),
                                     _eas,
                                 )?;
@@ -213,11 +179,7 @@ macro_rules! impl_host_function {
                         .try_lock()
                         .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?
                         .register_host_function(
-                            &HostFunctionDefinition::new(
-                                name.to_string(),
-                                parameter_types,
-                                R::TYPE,
-                            ),
+                            name.to_string(),
                             HyperlightFunction::new(func),
                         )?;
                 }

src/hyperlight_host/src/sandbox/host_funcs.rs

@@ -14,53 +14,33 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+use std::collections::HashMap;
 use std::io::{IsTerminal, Write};
 
 use hyperlight_common::flatbuffer_wrappers::function_types::{ParameterValue, ReturnValue};
 use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};
 use tracing::{instrument, Span};
 
-use super::{ExtraAllowedSyscall, FunctionsMap};
-use crate::func::host_functions::HostFunctionDefinition;
+use super::ExtraAllowedSyscall;
 use crate::func::HyperlightFunction;
 use crate::HyperlightError::HostFunctionNotFound;
 use crate::{new_error, Result};
 
-type HostFunctionDetails = Option<Vec<HostFunctionDefinition>>;
-
 #[derive(Default, Clone)]
 /// A Wrapper around details of functions exposed by the Host
 pub struct HostFuncsWrapper {
-    functions_map: FunctionsMap,
-    function_details: HostFunctionDetails,
+    functions_map: HashMap<String, (HyperlightFunction, Option<Vec<ExtraAllowedSyscall>>)>,
 }
 
 impl HostFuncsWrapper {
-    #[instrument(skip_all, parent = Span::current(), level = "Trace")]
-    fn get_host_funcs(&self) -> &FunctionsMap {
-        &self.functions_map
-    }
-    #[instrument(skip_all, parent = Span::current(), level = "Trace")]
-    fn get_host_funcs_mut(&mut self) -> &mut FunctionsMap {
-        &mut self.functions_map
-    }
-    #[instrument(skip_all, parent = Span::current(), level = "Trace")]
-    fn get_host_func_details(&self) -> &HostFunctionDetails {
-        &self.function_details
-    }
-    #[instrument(skip_all, parent = Span::current(), level = "Trace")]
-    fn get_host_func_details_mut(&mut self) -> &mut HostFunctionDetails {
-        &mut self.function_details
-    }
-
     /// Register a host function with the sandbox.
     #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")]
     pub(crate) fn register_host_function(
         &mut self,
-        hfd: &HostFunctionDefinition,
+        name: String,
         func: HyperlightFunction,
     ) -> Result<()> {
-        register_host_function_helper(self, hfd, func, None)
+        register_host_function_helper(self, name, func, None)
     }
 
     /// Register a host function with the sandbox, with a list of extra syscalls
@@ -69,11 +49,11 @@ impl HostFuncsWrapper {
     #[cfg(all(feature = "seccomp", target_os = "linux"))]
     pub(crate) fn register_host_function_with_syscalls(
         &mut self,
-        hfd: &HostFunctionDefinition,
+        name: String,
         func: HyperlightFunction,
         extra_allowed_syscalls: Vec<ExtraAllowedSyscall>,
     ) -> Result<()> {
-        register_host_function_helper(self, hfd, func, Some(extra_allowed_syscalls))
+        register_host_function_helper(self, name, func, Some(extra_allowed_syscalls))
     }
 
     /// Assuming a host function called `"HostPrint"` exists, and takes a
@@ -84,7 +64,7 @@ impl HostFuncsWrapper {
     #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")]
     pub(super) fn host_print(&mut self, msg: String) -> Result<i32> {
         let res = call_host_func_impl(
-            self.get_host_funcs(),
+            &self.functions_map,
             "HostPrint",
             vec![ParameterValue::String(msg)],
         )?;
@@ -104,56 +84,40 @@ impl HostFuncsWrapper {
         name: &str,
         args: Vec<ParameterValue>,
     ) -> Result<ReturnValue> {
-        call_host_func_impl(self.get_host_funcs(), name, args)
-    }
-
-    /// Insert a host function into the list of registered host functions.
-    pub(super) fn insert_host_function(&mut self, host_function: HostFunctionDefinition) {
-        match &mut self.function_details {
-            Some(host_functions) => host_functions.push(host_function),
-            None => {
-                let host_functions = Vec::from(&[host_function]);
-                self.function_details = Some(host_functions);
-            }
-        }
+        call_host_func_impl(&self.functions_map, name, args)
     }
 }
 
 fn register_host_function_helper(
     self_: &mut HostFuncsWrapper,
-    hfd: &HostFunctionDefinition,
+    name: String,
     func: HyperlightFunction,
     extra_allowed_syscalls: Option<Vec<ExtraAllowedSyscall>>,
 ) -> Result<()> {
     if let Some(_syscalls) = extra_allowed_syscalls {
         #[cfg(all(feature = "seccomp", target_os = "linux"))]
-        self_
-            .get_host_funcs_mut()
-            .insert(hfd.function_name.to_string(), func, Some(_syscalls));
+        self_.functions_map.insert(name, (func, Some(_syscalls)));
 
         #[cfg(not(all(feature = "seccomp", target_os = "linux")))]
         return Err(new_error!(
             "Extra syscalls are only supported on Linux with seccomp"
         ));
     } else {
-        self_
-            .get_host_funcs_mut()
-            .insert(hfd.function_name.to_string(), func, None);
+        self_.functions_map.insert(name, (func, None));
     }
-    self_.insert_host_function(hfd.clone());
 
     Ok(())
 }
 
 #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")]
 fn call_host_func_impl(
-    host_funcs: &FunctionsMap,
+    host_funcs: &HashMap<String, (HyperlightFunction, Option<Vec<ExtraAllowedSyscall>>)>,
     name: &str,
     args: Vec<ParameterValue>,
 ) -> Result<ReturnValue> {
     // Inner function containing the common logic
     fn call_func(
-        host_funcs: &FunctionsMap,
+        host_funcs: &HashMap<String, (HyperlightFunction, Option<Vec<ExtraAllowedSyscall>>)>,
         name: &str,
         args: Vec<ParameterValue>,
     ) -> Result<ReturnValue> {

src/hyperlight_host/src/sandbox/mod.rs

@@ -45,8 +45,6 @@ pub mod uninitialized;
 /// initialized `Sandbox`es.
 pub(crate) mod uninitialized_evolve;
 
-use std::collections::HashMap;
-
 /// Re-export for `SandboxConfiguration` type
 pub use config::SandboxConfiguration;
 /// Re-export for the `MultiUseSandbox` type
@@ -60,7 +58,6 @@ pub use uninitialized::GuestBinary;
 pub use uninitialized::UninitializedSandbox;
 
 use self::mem_mgr::MemMgrWrapper;
-use crate::func::HyperlightFunction;
 use crate::hypervisor::hypervisor_handler::HypervisorHandler;
 #[cfg(target_os = "windows")]
 use crate::hypervisor::windows_hypervisor_platform;
@@ -86,49 +83,6 @@ pub fn is_supported_platform() -> bool {
 /// Alias for the type of extra allowed syscalls.
 pub type ExtraAllowedSyscall = i64;
 
-/// A `HashMap` to map function names to `HyperlightFunction`s and their extra allowed syscalls.
-///
-/// Note: you cannot add extra syscalls on Windows, but the field is still present to avoid a funky
-/// conditional compilation setup. This isn't a big deal as this struct isn't public facing.
-#[derive(Clone, Default)]
-pub(super) struct FunctionsMap(
-    HashMap<String, (HyperlightFunction, Option<Vec<ExtraAllowedSyscall>>)>,
-);
-
-impl FunctionsMap {
-    /// Insert a new entry into the map
-    pub(super) fn insert(
-        &mut self,
-        key: String,
-        value: HyperlightFunction,
-        extra_syscalls: Option<Vec<ExtraAllowedSyscall>>,
-    ) {
-        self.0.insert(key, (value, extra_syscalls));
-    }
-
-    /// Get the value associated with the given key, if it exists.
-    pub(super) fn get(
-        &self,
-        key: &str,
-    ) -> Option<&(HyperlightFunction, Option<Vec<ExtraAllowedSyscall>>)> {
-        self.0.get(key)
-    }
-
-    /// Get the length of the map.
-    fn len(&self) -> usize {
-        self.0.len()
-    }
-}
-
-impl PartialEq for FunctionsMap {
-    #[instrument(skip_all, parent = Span::current(), level= "Trace")]
-    fn eq(&self, other: &Self) -> bool {
-        self.len() == other.len() && self.0.keys().all(|k| other.0.contains_key(k))
-    }
-}
-
-impl Eq for FunctionsMap {}
-
 /// Determine whether a suitable hypervisor is available to run
 /// this sandbox.
 ///