windows: Small clean ups, less allocs (#8688)

* Remove a couple of unneeded NUL checks since WideCString handles them
  already
* Amortize a few allocs

crates/host_env/src/windows.rs

@@ -1,10 +1,6 @@
 use rustpython_wtf8::Wtf8;
-use std::{
-    ffi::{OsStr, OsString},
-    io,
-    os::windows::ffi::OsStringExt,
-};
-use widestring::WideCString;
+use std::{ffi::OsStr, io};
+use widestring::{WideCStr, WideCString};
 use windows_sys::{
     Win32::{
         Foundation::{
@@ -28,6 +24,8 @@ use windows_sys::{
     w,
 };
 
+const NUL_ERROR: &str = "embedded null character";
+
 /// _MAX_ENV from Windows CRT stdlib.h - maximum environment variable size
 pub const _MAX_ENV: usize = 32767;
 pub const HRESULT_E_POINTER: i32 = E_POINTER;
@@ -218,18 +216,12 @@ pub fn get_windows_version() -> io::Result<WindowsVersionInfo> {
     }
     .check_win32_bool()?;
 
-    let service_pack = {
-        let (last, _) = version
-            .szCSDVersion
-            .iter()
-            .take_while(|&x| x != &0)
-            .enumerate()
-            .last()
-            .unwrap_or((0, &0));
-        let sp = OsString::from_wide(&version.szCSDVersion[..last]);
-        sp.into_string()
-            .map_err(|_| io::Error::other("service pack is not ASCII"))?
-    };
+    // szCSDVersion is at most 128 wide chars and represented as slice so overflow is impossible.
+    let service_pack = WideCStr::from_slice_truncate(&version.szCSDVersion)
+        .map_err(|_| io::Error::other(NUL_ERROR))?
+        .to_string()
+        .map_err(|_| io::Error::other("service pack is not ASCII"))?;
+
     let (major, minor, build) = get_kernel32_version()?;
     Ok(WindowsVersionInfo {
         major,
@@ -413,7 +405,7 @@ where
     T: AsRef<OsStr>,
 {
     fn to_wide_cstring(&self) -> Result<WideCString, io::Error> {
-        WideCString::from_os_str(self).map_err(|_| io::Error::other("embedded null character"))
+        WideCString::from_os_str(self).map_err(|_| io::Error::other(NUL_ERROR))
     }
 }
 
@@ -426,11 +418,11 @@ impl ToWideString for Wtf8 {
         //
         // For the sake of that behavior, fail on trailing NUL.
         if self.as_bytes().last().is_some_and(|&b| b == 0) {
-            return Err(io::Error::other("embedded null character"));
+            return Err(io::Error::other(NUL_ERROR));
         }
 
         let mut buf = Vec::with_capacity(self.len() + 1);
         buf.extend(self.encode_wide());
-        WideCString::from_vec(buf).map_err(|_| io::Error::other("embedded null character"))
+        WideCString::from_vec(buf).map_err(|_| io::Error::other(NUL_ERROR))
     }
 }

crates/vm/src/stdlib/nt.rs

@@ -10,7 +10,7 @@ pub(crate) mod module {
         builtins::{PyBytes, PyCapsule, PyDictRef, PyListRef, PyStr, PyStrRef, PyTupleRef},
         convert::ToPyException,
         exceptions::{self, OSErrorBuilder, ToOSErrorBuilder},
-        function::{ArgMapping, Either, OptionalArg},
+        function::{ArgMapping, Either, FsPath, OptionalArg},
         host_env::crt_fd,
         ospath::{OsPath, OsPathOrFd},
         stdlib::os::{_os, DirFd, SupportFunc, SymlinkArgs},
@@ -418,8 +418,6 @@ pub(crate) mod module {
         argv: Either<PyListRef, PyTupleRef>,
         vm: &VirtualMachine,
     ) -> PyResult<intptr_t> {
-        use crate::function::FsPath;
-
         let path = path.to_wide_cstring(vm)?;
 
         let argv = vm.extract_elements_with(argv.as_ref(), |obj| {
@@ -448,8 +446,6 @@ pub(crate) mod module {
         env: PyDictRef,
         vm: &VirtualMachine,
     ) -> PyResult<intptr_t> {
-        use crate::function::FsPath;
-
         let path = path.to_wide_cstring(vm)?;
 
         let argv = vm.extract_elements_with(argv.as_ref(), |obj| {
@@ -466,7 +462,7 @@ pub(crate) mod module {
         }
 
         // Build environment strings as "KEY=VALUE\0" wide strings
-        let mut env_strings: Vec<widestring::WideCString> = Vec::new();
+        let mut env_strings: Vec<widestring::WideCString> = Vec::with_capacity(env.size().used);
         for (key, value) in env {
             let key = FsPath::try_from_path_like(key, true, vm)?;
             let value = FsPath::try_from_path_like(value, true, vm)?;
@@ -482,7 +478,7 @@ pub(crate) mod module {
 
             let env_str = format!("{key_str}={value_str}");
             env_strings.push(
-                widestring::WideCString::from_os_str(&*std::ffi::OsString::from(env_str))
+                widestring::WideCString::from_str(&env_str)
                     .map_err(|err| err.to_pyexception(vm))?,
             );
         }
@@ -505,14 +501,12 @@ pub(crate) mod module {
                 vm.new_runtime_error("exec not supported for isolated subinterpreters".to_owned())
             );
         }
-        let make_widestring =
-            |s: &str| widestring::WideCString::from_os_str(s).map_err(|err| err.to_pyexception(vm));
 
         let path = path.to_wide_cstring(vm)?;
-
         let argv = vm.extract_elements_with(argv.as_ref(), |obj| {
             let arg = PyStrRef::try_from_object(vm, obj)?;
-            make_widestring(arg.expect_str())
+            widestring::WideCString::from_str(arg.expect_str())
+                .map_err(|err| err.to_pyexception(vm))
         })?;
 
         let first = argv
@@ -540,14 +534,12 @@ pub(crate) mod module {
                 vm.new_runtime_error("exec not supported for isolated subinterpreters".to_owned())
             );
         }
-        let make_widestring =
-            |s: &str| widestring::WideCString::from_os_str(s).map_err(|err| err.to_pyexception(vm));
 
         let path = path.to_wide_cstring(vm)?;
-
         let argv = vm.extract_elements_with(argv.as_ref(), |obj| {
             let arg = PyStrRef::try_from_object(vm, obj)?;
-            make_widestring(arg.expect_str())
+            widestring::WideCString::from_str(arg.expect_str())
+                .map_err(|err| err.to_pyexception(vm))
         })?;
 
         let first = argv
@@ -572,6 +564,7 @@ pub(crate) mod module {
                 cold_path();
                 return Err(exceptions::nul_char_error(vm));
             }
+
             // Validate: empty key or '=' in key after position 0
             // (search from index 1 because on Windows starting '=' is allowed
             // for defining hidden environment variables)
@@ -580,7 +573,9 @@ pub(crate) mod module {
             }
 
             let env_str = format!("{key_str}={value_str}");
-            env_strings.push(make_widestring(&env_str)?);
+            // SAFETY: Interior NULs are rejected above. WideCString appends the trailing NUL.
+            let env_str = unsafe { widestring::WideCString::from_str_unchecked(&env_str) };
+            env_strings.push(env_str);
         }
 
         let argv_refs: Vec<&widestring::WideCStr> = argv.iter().map(|s| s.as_ref()).collect();