move more constructor stuff into stringarray

crates/wasi-common/src/ctx.rs

@@ -1,22 +1,21 @@
 use crate::entry::{Entry, EntryHandle};
 use crate::fdpool::FdPool;
 use crate::handle::Handle;
-use crate::string_array::{StringArray, StringArrayError};
+use crate::string_array::{PendingString, StringArray, StringArrayError};
 use crate::sys::osdir::OsDir;
 use crate::sys::stdio::NullDevice;
 use crate::sys::stdio::{Stderr, StderrExt, Stdin, StdinExt, Stdout, StdoutExt};
 use crate::virtfs::{VirtualDir, VirtualDirEntry};
 use crate::wasi::types;
-use crate::{Error, Result};
+use crate::Error;
 use std::borrow::Borrow;
 use std::cell::RefCell;
 use std::collections::HashMap;
 use std::convert::TryFrom;
-use std::ffi::OsString;
 use std::fs::File;
 use std::path::{Path, PathBuf};
 use std::rc::Rc;
-use std::{env, io, string};
+use std::{env, io};
 
 /// Possible errors when `WasiCtxBuilder` fails building
 /// `WasiCtx`.
@@ -25,14 +24,6 @@ pub enum WasiCtxBuilderError {
     /// General I/O error was encountered.
     #[error("general I/O error encountered: {0}")]
     Io(#[from] io::Error),
-    /// Provided sequence of bytes was not a valid UTF-8.
-    #[error("provided sequence is not valid UTF-8: {0}")]
-    InvalidUtf8(#[from] string::FromUtf8Error),
-    /// Provided sequence of bytes was not a valid UTF-16.
-    ///
-    /// This error is expected to only occur on Windows hosts.
-    #[error("provided sequence is not valid UTF-16: {0}")]
-    InvalidUtf16(#[from] string::FromUtf16Error),
     /// Error constructing arguments
     #[error("while constructing arguments: {0}")]
     Args(#[source] StringArrayError),
@@ -67,44 +58,6 @@ impl std::fmt::Debug for PendingEntry {
     }
 }
 
-#[derive(Debug, Eq, Hash, PartialEq)]
-enum PendingString {
-    Bytes(Vec<u8>),
-    OsString(OsString),
-}
-
-impl From<Vec<u8>> for PendingString {
-    fn from(bytes: Vec<u8>) -> Self {
-        Self::Bytes(bytes)
-    }
-}
-
-impl From<OsString> for PendingString {
-    fn from(s: OsString) -> Self {
-        Self::OsString(s)
-    }
-}
-
-impl PendingString {
-    fn into_string(self) -> WasiCtxBuilderResult<String> {
-        let res = match self {
-            Self::Bytes(v) => String::from_utf8(v)?,
-            #[cfg(unix)]
-            Self::OsString(s) => {
-                use std::os::unix::ffi::OsStringExt;
-                String::from_utf8(s.into_vec())?
-            }
-            #[cfg(windows)]
-            Self::OsString(s) => {
-                use std::os::windows::ffi::OsStrExt;
-                let bytes: Vec<u16> = s.encode_wide().collect();
-                String::from_utf16(&bytes)?
-            }
-        };
-        Ok(res)
-    }
-}
-
 struct PendingPreopen(Box<dyn FnOnce() -> WasiCtxBuilderResult<Box<dyn Handle>>>);
 
 impl PendingPreopen {
@@ -322,30 +275,11 @@ impl WasiCtxBuilder {
     pub fn build(&mut self) -> WasiCtxBuilderResult<WasiCtx> {
         // Process arguments and environment variables into `String`s, failing quickly if they
         // contain any NUL bytes, or if conversion from `OsString` fails.
-        let args = self
+        let args =
-            .args
+            StringArray::from_pending_vec(self.args.take().expect("WasiCtxBuilder has args"))
-            .take()
+                .map_err(WasiCtxBuilderError::Args)?;
-            .unwrap()
+        let env = StringArray::from_pending_map(self.env.take().expect("WasiCtxBuilder has env"))
-            .into_iter()
+            .map_err(WasiCtxBuilderError::Env)?;
-            .map(|arg| arg.into_string())
-            .collect::<WasiCtxBuilderResult<Vec<String>>>()?;
-        let args = StringArray::new(args).map_err(WasiCtxBuilderError::Args)?;
-        let env = self
-            .env
-            .take()
-            .unwrap()
-            .into_iter()
-            .map(|(k, v)| {
-                k.into_string().and_then(|mut pair| {
-                    v.into_string().and_then(|v| {
-                        pair.push('=');
-                        pair.push_str(v.as_str());
-                        Ok(pair)
-                    })
-                })
-            })
-            .collect::<WasiCtxBuilderResult<Vec<String>>>()?;
-        let env = StringArray::new(env).map_err(WasiCtxBuilderError::Env)?;
 
         let mut entries = EntryTable::new();
         // Populate the non-preopen entries.
@@ -461,7 +395,7 @@ impl WasiCtx {
     }
 
     /// Get an immutable `Entry` corresponding to the specified raw WASI `fd`.
-    pub(crate) fn get_entry(&self, fd: types::Fd) -> Result<Rc<Entry>> {
+    pub(crate) fn get_entry(&self, fd: types::Fd) -> Result<Rc<Entry>, Error> {
         match self.entries.borrow().get(&fd) {
             Some(entry) => Ok(entry),
             None => Err(Error::Badf),
@@ -472,7 +406,7 @@ impl WasiCtx {
     ///
     /// The `Entry` will automatically get another free raw WASI `fd` assigned. Note that
     /// the two subsequent free raw WASI `fd`s do not have to be stored contiguously.
-    pub(crate) fn insert_entry(&self, entry: Entry) -> Result<types::Fd> {
+    pub(crate) fn insert_entry(&self, entry: Entry) -> Result<types::Fd, Error> {
         self.entries.borrow_mut().insert(entry).ok_or(Error::Mfile)
     }
 
@@ -483,7 +417,7 @@ impl WasiCtx {
     }
 
     /// Remove `Entry` corresponding to the specified raw WASI `fd` from the `WasiCtx` object.
-    pub(crate) fn remove_entry(&self, fd: types::Fd) -> Result<Rc<Entry>> {
+    pub(crate) fn remove_entry(&self, fd: types::Fd) -> Result<Rc<Entry>, Error> {
         self.entries.borrow_mut().remove(fd).ok_or(Error::Badf)
     }
 

crates/wasi-common/src/string_array.rs

@@ -1,12 +1,45 @@
 use crate::Error;
+use std::collections::HashMap;
 use std::convert::TryInto;
-use std::ffi::CString;
+use std::ffi::{CString, OsString};
 use wiggle::GuestPtr;
 
-pub struct StringArray {
+#[derive(Debug, Eq, Hash, PartialEq)]
-    elems: Vec<CString>,
+pub enum PendingString {
-    pub number_elements: u32,
+    Bytes(Vec<u8>),
-    pub cumulative_size: u32,
+    OsString(OsString),
+}
+
+impl From<Vec<u8>> for PendingString {
+    fn from(bytes: Vec<u8>) -> Self {
+        Self::Bytes(bytes)
+    }
+}
+
+impl From<OsString> for PendingString {
+    fn from(s: OsString) -> Self {
+        Self::OsString(s)
+    }
+}
+
+impl PendingString {
+    pub fn into_string(self) -> Result<String, StringArrayError> {
+        let res = match self {
+            Self::Bytes(v) => String::from_utf8(v)?,
+            #[cfg(unix)]
+            Self::OsString(s) => {
+                use std::os::unix::ffi::OsStringExt;
+                String::from_utf8(s.into_vec())?
+            }
+            #[cfg(windows)]
+            Self::OsString(s) => {
+                use std::os::windows::ffi::OsStrExt;
+                let bytes: Vec<u16> = s.encode_wide().collect();
+                String::from_utf16(&bytes)?
+            }
+        };
+        Ok(res)
+    }
 }
 
 #[derive(Debug, thiserror::Error)]
@@ -23,10 +56,42 @@ pub enum StringArrayError {
     /// Cumulative element size: must fit into u32
     #[error("cumulative element size too big")]
     CumElemSize,
+    /// Provided sequence of bytes was not a valid UTF-8.
+    #[error("provided sequence is not valid UTF-8: {0}")]
+    InvalidUtf8(#[from] std::string::FromUtf8Error),
+    /// Provided sequence of bytes was not a valid UTF-16.
+    ///
+    /// This error is expected to only occur on Windows hosts.
+    #[error("provided sequence is not valid UTF-16: {0}")]
+    InvalidUtf16(#[from] std::string::FromUtf16Error),
 }
 
+pub struct StringArray {
+    elems: Vec<CString>,
+    pub number_elements: u32,
+    pub cumulative_size: u32,
+}
 impl StringArray {
-    pub fn new(elems: Vec<String>) -> Result<Self, StringArrayError> {
+    pub fn from_pending_vec(elems: Vec<PendingString>) -> Result<Self, StringArrayError> {
+        let elems = elems
+            .into_iter()
+            .map(|arg| arg.into_string())
+            .collect::<Result<Vec<String>, StringArrayError>>()?;
+        Self::from_strings(elems)
+    }
+    pub fn from_pending_map(
+        elems: HashMap<PendingString, PendingString>,
+    ) -> Result<Self, StringArrayError> {
+        let mut pairs = Vec::new();
+        for (k, v) in elems.into_iter() {
+            let mut pair = k.into_string()?;
+            pair.push('=');
+            pair.push_str(&v.into_string()?);
+            pairs.push(pair);
+        }
+        Self::from_strings(pairs)
+    }
+    pub fn from_strings(elems: Vec<String>) -> Result<Self, StringArrayError> {
         let elems = elems
             .into_iter()
             .map(|s| CString::new(s))