Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/host_env/src/select.rs`:
- Around line 171-176: sec_to_timeval can produce negative tv_usec when sec is
negative which yields invalid timeval for select(); update sec_to_timeval so
negative timeouts are handled by clamping to a non-negative timeval (e.g., if
sec <= 0.0 return timeval { tv_sec: 0, tv_usec: 0 }), otherwise compute tv_sec =
sec.trunc() as _ and tv_usec = (sec.fract() * 1e6) as _ (ensuring tv_usec is
non-negative and fits the timeval field); reference sec_to_timeval and timeval
(and select()) when making the change.

In `@crates/host_env/src/shm.rs`:
- Around line 5-6: Replace the infallible conversion using try_into().unwrap()
with a direct cast on FreeBSD: change the conversion of mode in the
#[cfg(target_os = "freebsd")] branch so that the local variable `mode` is cast
to the target type (since `libc::c_uint` and `libc::mode_t` are both u32 on
FreeBSD) rather than calling `try_into().unwrap()`, updating the `mode` binding
used later in functions in this module (shm.rs) accordingly.

In `@crates/host_env/src/syslog.rs`:
- Around line 62-64: The implementation of pub const fn log_mask(pri: i32)
shifts the wrong operand; change the shift to compute 1 << pri (not pri << 1) so
LOG_MASK(0) yields 1 and all masks are correct. Update log_mask to perform the
shift with a literal one of the correct integer type (e.g., 1i32 << pri or (1 as
i32) << pri) and keep the function signature and const semantics intact. Ensure
the return type stays i32 and handle pri as-is (no additional checks needed
here).
- Around line 35-43: syslog() and closelog() have race conditions: ensure
syslog() holds the global_ident read lock while calling libc::syslog() so the
ident Box<CStr> cannot be freed concurrently, and change closelog() to acquire
the global_ident write lock and perform the is_open() check and drop the ident
while holding that write lock (preventing an openlog() from racing in between).
Also correct log_mask() to use the POSIX LOG_MASK formula (return 1 << pri, not
pri << 1). Update references: openlog (stores GlobalIdent), syslog
(acquire/read-lock around libc::syslog call), closelog (do is_open check and
drop under write lock), global_ident (use its locks accordingly), and log_mask
(use 1 << pri).

In `@crates/host_env/src/time.rs`:
- Around line 24-27: get_tz_info currently ignores GetTimeZoneInformation's
return value and returns a zeroed TIME_ZONE_INFORMATION on failure; change
get_tz_info to return a
Result<windows_sys::Win32::System::Time::TIME_ZONE_INFORMATION, std::io::Error>
(or similar error type), call GetTimeZoneInformation and check its return value
against TIME_ZONE_ID_INVALID, and on failure call GetLastError to construct and
return an Err(std::io::Error::from_raw_os_error(...)); update all callers in
crates/vm/src/stdlib/time.rs (the wrapper function and the functions altzone,
timezone, daylight, tzname and the platform module) to handle the Result by
propagating errors or mapping the Ok value to the previous behavior so failures
are not silently converted to a zeroed struct. Ensure unique symbols:
get_tz_info, wrapper function in stdlib/time.rs, altzone, timezone, daylight,
tzname, and the platform module are updated accordingly.
- Around line 31-45: checked_tm_from_struct_time currently only checks
tm.tm_wday < 0 causing a potential out-of-bounds when asctime_from_tm indexes
WDAY_NAME; update checked_tm_from_struct_time to validate the full range for
tm.tm_wday (>=0 && <=6) and ensure tm.tm_mon is validated for 0..=11 as well so
both WDAY_NAME[tm.tm_wday as usize] and MON_NAME[tm.tm_mon as usize] cannot
panic; locate and update the checks around tm.tm_wday and tm.tm_mon in the
checked_tm_from_struct_time function accordingly.

In `@crates/stdlib/src/openssl.rs`:
- Around line 1671-1681: The fallback arm of the map_err on the
rustpython_host_env::fileutils::fopen call currently returns
vm.new_os_error(e.to_string()) which drops the original errno and prevents
mapping to specific OSError subtypes; change that fallback to return the
original IO error converted to a Python exception by calling
e.to_pyexception(vm) so errno is preserved (i.e., replace
vm.new_os_error(e.to_string()) with e.to_pyexception(vm) inside the map_err
closure).

In `@crates/stdlib/src/select.rs`:
- Around line 106-108: The select path is reading the wrong Windows error
namespace; update the error retrieval in the host_select::select() helper
(crates/host_env/src/select.rs) to call WSAGetLastError() on Windows and
construct the io::Error from that code instead of using
GetLastError()/io::Error::last_os_error(); ensure the returned Error codes
propagate into select.rs::select() and socket.rs::sock_select() so the
interrupted-retry logic can detect Winsock-specific errors (e.g.,
WSAEINTR/WSAEINPROGRESS) correctly. Use conditional compilation for Windows
targets to call the WinSock API (WSAGetLastError) and fallback to the existing
path on non-Windows platforms so behavior is unchanged elsewhere.

In `@crates/vm/src/stdlib/msvcrt.rs`:
- Around line 46-54: The wide-char wrappers (getwch, getche, getwche) currently
treat MSVCRT UTF-16 code units as Rust char and can panic on surrogate halves;
update the plumbing to handle UTF-16 code units instead of Unicode scalars:
change the host_msvcrt side bindings for _getwch/_getwche/_putwch/_ungetwch to
use u16/u32 (i.e., raw UTF-16 code units), stop calling char::from_u32 on the
returned values, and in the VM wrappers (getwch, getche, getwche) either return
the raw u16 code units (Vec<u16>) or safely decode sequences using
std::char::decode_utf16 to produce a String without panicking (handling Err by
using replacement U+FFFD); likewise, when sending to putwch/ungetwch, accept
UTF-16 code units (u16/u32) rather than truncating Rust char.

---

Nitpick comments:
In `@crates/host_env/src/fcntl.rs`:
- Around line 57-75: The function lockf currently mixes flock-style flags
(checks libc::LOCK_NB on the cmd parameter) with fcntl operations (using
F_SETLK/F_SETLKW), which is confusing and potentially incorrect; either rename
the function to something like fcntl_lock (or fcntl_lockf) and update its
docstring to state it expects flock-style nonblocking flag semantics, or change
the cmd handling to accept POSIX lockf commands (F_LOCK/F_TLOCK/F_ULOCK/F_TEST)
and map them appropriately to F_SETLK/F_SETLKW; locate the function named lockf
and its cmd parameter and adjust the name and/or the flag checks and
documentation accordingly so callers aren’t misled by the mixed APIs.

In `@crates/host_env/src/msvcrt.rs`:
- Around line 58-60: The putwch function silently truncates non-BMP characters
when casting char to u16; locate putwch (and its call to
suppress_iph!(_putwch(...))) and add validation or documentation: check if c as
u32 <= 0xFFFF before calling _putwch, and if not either map to a replacement
(e.g. '\u{FFFD}') or return/propagate an error/log a warning (choose consistent
behavior with your crate's error handling); alternatively add a clear doc
comment on putwch stating it accepts only BMP characters and that callers must
handle supplementary-plane code points themselves.
- Around line 36-52: The current implementations of getwch and getwche panic if
char::from_u32 returns None (surrogate/invalid codepoints); to avoid this, stop
panicking and normalize invalid values to the Unicode replacement character
instead so the API stays unchanged: in getwch and getwche replace the
unwrap_or_else panic path with a fallback to '\u{FFFD}' before .to_string();
keep getche as-is (it returns raw bytes). Ensure you remove the panic messages
and any panic-causing closures in functions getwch and getwche so invalid input
yields U+FFFD rather than aborting.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/host_env/src/ctypes.rs`:
- Around line 1883-1904: The function copy_com_pointer currently uses
debug_assert! to validate the IUnknown vtable which is removed in release
builds, causing UB when src_ptr is non-zero but vtable is null or bogus; change
the runtime checks so that after computing iunknown and vtable you explicitly
test if vtable.is_null() (and also check the function pointer at vtable.add(1)
is non-null) and return HRESULT_E_POINTER if invalid, and only then perform the
transmute and call addref_fn; ensure you do not dereference *vtable until those
runtime checks pass and likewise avoid transmuting a null function pointer.
- Around line 375-402: The code in wstring_from_bytes and
wchar_array_field_value treats each WCHAR as an independent Unicode scalar via
char::from_u32, which drops surrogate pairs; instead collect the sequence of u16
code units produced by wchar_from_bytes (using WCHAR_SIZE to read each unit),
stop at a terminating 0, then decode the whole Vec<u16> as UTF-16 using either
String::from_utf16 or std::char::decode_utf16 to produce the String (use
decode_utf16 to gracefully handle invalid/ lone surrogates if you want
replacement chars); update both wstring_from_bytes and wchar_array_field_value
to build a Vec<u16> and run the proper UTF-16 decoder rather than mapping each
code unit to char individually.
- Around line 1981-1991: The function call_result_bytes currently hard-codes the
reported size for CallResult::Value as core::mem::size_of::<i64>(), which can be
wrong on some targets; instead use the actual serialized byte length produced by
to_ne_bytes(). Update call_result_bytes (both CallResult::Pointer and
CallResult::Value branches) to compute the reported width from the length of the
bytes returned by ptr.to_ne_bytes()/val.to_ne_bytes() (e.g., use bytes.len() or
equivalent) and return that length instead of a hard-coded type size.
- Around line 208-210: simple_type_align currently returns simple_type_size
which is wrong for ABIs where alignment != size; replace the forwarding with a
real alignment mapping inside simple_type_align: match on the type strings used
elsewhere (e.g. the same tokens handled by simple_type_size) and return the C
alignment for each token (e.g. i8/u8 => 1, i16/u16 => 2, i32/u32/ f32 => 4,
i64/u64/f64 => 8, ptr types => pointer alignment based on target pointer width,
etc.), returning Some(alignment) for recognized types and None as the fallback
(or use simple_type_size only when appropriate); reference the function name
simple_type_align and reuse simple_type_size only as a fallback, not as the
primary alignment source.
- Around line 1278-1298: The match arms in write_callback_result() (and
similarly in read_value_at_address() at the other noted location) currently
treat "l"/"L" as fixed 64-bit/32-bit types causing truncation/overrun on
ILP32/LP64 targets; change the memory access to use libc::c_long and
libc::c_ulong for the "l" and "L" branches (use *(result as *mut libc::c_long)
and *(result as *mut libc::c_ulong) respectively) and only cast between
CallbackResultValue's integer variants and libc::c_long/c_ulong at the API
boundary so the in-memory read/write respects the platform c_long/c_ulong width.
- Around line 102-115: with_swapped_errno currently writes the ctypes-local
errno into the OS slot, calls f, stores the post-call errno into
CTYPES_LOCAL_ERRNO, but fails to restore the original OS errno so the embedding
thread sees the foreign function's errno; fix by capturing the original OS errno
before setting it (save_old_os = crate::os::get_errno()), set the OS errno to
the ctypes-local value as now, call f, capture new_error =
crate::os::get_errno(), restore the original OS errno
(crate::os::set_errno(save_old_os)), then set CTYPES_LOCAL_ERRNO to new_error
and return result; apply the exact same pattern to the other helper in the
153-165 region (the corresponding swapped LastError helper) using
crate::os::get_last_error()/set_last_error() and the CTYPES_LOCAL_LAST_ERROR
slot.
- Around line 707-775: In read_array_element, guard against slicing
buffer[offset..] before verifying offset and available bytes: for type "u"
return DecodedValue::String(String::new()) if offset >= buffer.len(); for "f"
return DecodedValue::Float(0.0) if offset + 4 > buffer.len(); for "d"/"g" return
DecodedValue::Float(0.0) if offset + 8 > buffer.len(); and in the
integer/default branch use buffer.get(offset..) (or check offset + element_size
<= buffer.len()) before calling get(..element_size) and int_from_bytes so you
return the existing Signed(0) fallback instead of panicking; update the checks
around usages of wchar_from_bytes, first_chunk::<4>(), first_chunk::<8>(), and
int_from_bytes accordingly.
- Around line 172-184: LONG_DOUBLE_SIZE and the "g" format handlers are
inconsistent: LONG_DOUBLE_SIZE should represent the platform long double size
(16 bytes on x86_64/aarch64 Unix) and the "g" encoder/decoder and FFI mapping
must use the proper long double type instead of marshaling as an 8-byte f64.
Update the LONG_DOUBLE_SIZE constant to reflect c_longdouble size where
appropriate, change the "g" encoding path (the block that currently encodes as
8-byte float then pads) to encode a c_longdouble value of the correct size and
preserve its bytes (no zero-padding/truncation), change the "g" decoding path
(the block that currently reads 8 bytes into c_double) to read the full
c_longdouble size and reconstruct a c_longdouble, and replace any uses of
Type::f64() for the "g"/long double FFI mapping with the proper long double
mapping (Type::longdouble() / libffi ffi_type_longdouble) so libffi receives the
correct ffi_type_longdouble for FFI calls.
- Around line 2409-2416: The get_pointer() implementation in SharedLibrary must
not transmute the safe libloading::Library; instead persist a stable raw handle
or synthetic ID when the library is created and return that here. Modify the
SharedLibrary struct to either (A) capture the raw handle up front by converting
the platform-specific libloading::os::unix::Library to a raw handle via
into_raw()/from_raw() and store that usize (update creation/loading code to call
into_raw and reconstruct the safe wrapper with from_raw), or (B) add a dedicated
stable identifier field (AtomicUsize counter or UUID) assigned when the library
is loaded and return that from get_pointer(). Replace the transmute_copy usage
in get_pointer() to read and return the stored raw_handle or stable_id and
ensure destruction/reconstruction uses from_raw if you chose the raw-handle
approach.

In `@crates/host_env/src/faulthandler.rs`:
- Around line 196-209: The loop in enable_fatal_handlers iterates
FATAL_SIGNAL_HANDLERS and may return false after a failed install_sigaction,
leaving earlier entries enabled; update enable_fatal_handlers to roll back any
installs when a subsequent install fails by iterating the already-enabled
entries and restoring their previous handlers (using entry.previous and the same
mechanism as install_sigaction or its counterpart) and mark them disabled before
returning false; apply the same rollback logic to the analogous code region
(lines ~383-397) so partial installs are undone on failure.
- Around line 269-307: Validate the incoming signum before casting/indexing in
register_user_signal: check that the original signum (libc::c_int) is
non-negative and that (signum as usize) < USER_SIGNAL_CAPACITY; if not, return
an appropriate io::Error (e.g., from_raw_os_error(libc::EINVAL)) instead of
indexing USER_SIGNALS or USER_SIGNAL_CAPACITY. Use the validated usize only
after this check and keep references to USER_SIGNALS, USER_SIGNAL_CAPACITY, and
register_user_signal consistent.
- Around line 347-356: The code currently captures saved_errno after
restore_sigaction and then restores errno using errno_after_raise, which loses
the caller's original errno; move the capture of the caller's errno to before
you call restore_sigaction (call crate::os::get_errno() into saved_errno before
restore_sigaction), continue to call raise_signal and re-install the handler
(install_sigaction), then after re-installation call
crate::os::set_errno(saved_errno) instead of restoring errno_after_raise so the
original errno is preserved; update/remove the temporary errno_after_raise usage
accordingly and keep restore_sigaction, raise_signal, and install_sigaction
calls in the same sequence.
- Line 9: The static mutable FATAL_SIGNAL_HANDLERS must be protected from
concurrent access; replace the current static mut with a synchronization
primitive (e.g., wrap the underlying table in a Mutex or RwLock) and update the
public functions enable_fatal_handlers() and disable_fatal_handlers() to acquire
the lock before reading/modifying the table (or alternatively make those
functions unsafe and document the synchronization contract); ensure you
reference and lock the same guarded static (FATAL_SIGNAL_HANDLERS) in all helper
routines that touch it so no aliased mutable references or data races can occur.

In `@crates/host_env/src/io.rs`:
- Around line 216-218: The is_would_block_error function currently only checks
raw_os_error() == Some(libc::EAGAIN); change it to detect would-block using
Rust's platform-agnostic io::ErrorKind by checking err.kind() ==
io::ErrorKind::WouldBlock (and optionally OR with the existing raw_os_error() ==
Some(libc::EAGAIN) if you want to preserve the explicit errno check). Update the
function signature/reference to use std::io::ErrorKind so is_would_block_error
reliably returns true across platforms.

In `@crates/host_env/src/posix.rs`:
- Around line 457-468: The utimes function's tv closure incorrectly uses
d.as_micros() which gives total microseconds instead of the fractional
microseconds for timeval.tv_usec; update the tv closure in utimes so tv_sec
remains d.as_secs() as _ and tv_usec is the subsecond component (use
d.subsec_micros() or d.subsec_nanos()/1_000) cast to the correct type, ensuring
timeval.tv_usec contains only the microsecond remainder; adjust only the tv
closure inside the utimes function (references: utimes, tv closure,
timeval.tv_sec and timeval.tv_usec).

In `@crates/host_env/src/select.rs`:
- Around line 267-277: The function wait clears events then calls
rustix::event::epoll::wait which writes into events' spare capacity but doesn't
update events.len(); update events to reflect the number of entries written
before returning. Specifically, in the Ok(n) arm of select::wait (the function
named wait, operating on events: &mut Vec<Event> and calling
rustix::event::epoll::wait), set the vector length to n (e.g., advance or
set_len) so callers see the populated entries, then return Ok(n).
- Around line 86-88: Check the FD_SETSIZE bound before modifying set.__nfds to
avoid temporarily leaving the struct inconsistent: when adding a file
descriptor, ensure you validate n = set.__nfds against FD_SETSIZE (e.g., if n >=
FD_SETSIZE return an error) before doing set.__nfds = n + 1 and set.__fds[n] =
fd; in other words, perform the FD_SETSIZE check first and only increment __nfds
and write to __fds after the bound check succeeds (referencing set.__nfds,
set.__fds, and FD_SETSIZE).

In `@crates/host_env/src/socket.rs`:
- Around line 530-531: Replace the incorrect alias of SO_EXCLUSIVEADDRUSE to
SO_REUSEADDR: update the constant SO_EXCLUSIVEADDRUSE so it uses the
Windows-specific numeric value -5 (0xFFFFFFFB) instead of referencing
SO_REUSEADDR; locate the declaration of pub const SO_EXCLUSIVEADDRUSE and change
its value to the correct integer literal for Windows.

In `@crates/host_env/src/time.rs`:
- Around line 138-149: The code calls libc::sysconf(libc::_SC_CLK_TCK) into
tick_for_second and then divides process time fields by it without checking for
non-positive/zero values; guard the syscall result by checking the raw return
(from libc::sysconf) for <= 0 (or error) before converting to f64, return an
Err(std::io::Error::new(...)) on invalid tick count, and only compute
ProcessTimes.user/system/children_user/children_system/elapsed after confirming
tick_for_second is positive; reference the tick_for_second variable, the
libc::sysconf(libc::_SC_CLK_TCK) call, and the ProcessTimes construction to
locate where to add the check and early return.

In `@crates/host_env/src/winapi.rs`:
- Around line 670-678: The branch that detects a SIGINT event has an off-by-one:
when sigint_event is pushed into thread_handles_raw it occupies index
thread_handles_raw.len() - 1, so change the comparison against result to use
WAIT_OBJECT_0 + (thread_handles_raw.len() - 1) instead of WAIT_OBJECT_0 +
thread_handles_raw.len(); update the condition that references sigint_event,
thread_handles_raw and result (and KEEP using WAIT_OBJECT_0 and
ERROR_CONTROL_C_EXIT) so it correctly matches the signaled index for the SIGINT
handle.

---

Outside diff comments:
In `@crates/host_env/src/msvcrt.rs`:
- Around line 42-54: The getwch and getwche functions currently call
char::from_u32(...).unwrap(), which can panic if the underlying unsafe calls
_getwch() or _getwche() return an invalid Unicode scalar (e.g., surrogate
values); change both to handle invalid code points by using
char::from_u32(value).unwrap_or(char::REPLACEMENT_CHARACTER) (or equivalent
fallback) before calling to_string(), and keep getche as-is (it returns raw u8)
or document its behavior; ensure you reference the unsafe calls _getwch and
_getwche and the wrapper functions getwch and getwche when making the change.

---

Minor comments:
In `@crates/host_env/src/nt.rs`:
- Around line 1546-1559: The getlogin function currently calls
OsString::from_wide(...).to_str().unwrap() which can panic if the username
contains non-UTF-8 sequences; change the conversion to a loss-tolerant form
(e.g., use OsString/OsStr -> to_string_lossy().into_owned()) so invalid UTF-8 is
replaced rather than panicking. Locate the getlogin function and replace the
to_str().unwrap().to_string() chain with a to_string_lossy().into_owned() call
on the OsString/OsStr produced from OsString::from_wide(&buffer[..(size - 1) as
usize]) to return a safe String.

In `@crates/host_env/src/overlapped.rs`:
- Line 536: The current code uses unwrap() on ACCEPT_EX.get() (and similar
CONNECT_EX.get(), etc.), which will panic if initialize_winsock_extensions() was
not called; change the API to return an io::Result (e.g., change the function
signature to io::Result<u32>) and replace unwrap() with a checked lookup that
returns Err(io::Error::new(...)) when the extension pointer is None, or
alternatively call initialize_winsock_extensions() lazily and return an error on
failure; reference ACCEPT_EX.get(), CONNECT_EX.get(), and
initialize_winsock_extensions() when making the change so the caller-visible
precondition is removed and errors are propagated instead of panicking.

In `@crates/host_env/src/signal.rs`:
- Around line 299-303: The write call is casting wakeup_fd to the C int
parameter which can truncate on 64-bit Windows; change the unsafe
libc::write(wakeup_fd as _, &sigbyte... ) usage to first attempt a safe
conversion like i32::try_from(wakeup_fd) and only call libc::write with the
converted fd, handling the Err case (e.g., skip the write and log or return) to
avoid truncation; reference the wakeup_fd, sigbyte, and the libc::write
invocation when making this change.

In `@crates/host_env/src/winapi.rs`:
- Around line 627-636: The CreateThread call currently passes a stack size of 1
(in the call that creates the thread for batch_wait_thread), which is
non-idiomatic and suspicious; change the third argument to 0 so the thread uses
the process/executable default stack size (i.e., replace the literal 1 with 0 in
the CreateThread invocation that passes batch_wait_thread and
Arc::as_ptr(data)).

In `@crates/host_env/src/windows.rs`:
- Around line 133-144: The slice used to build service_pack omits the last
non-null wchar because `last` is the index of that character; change the slice
passed to OsString::from_wide to include that character (use last + 1) when
calling from_wide on version.szCSDVersion so service_pack contains the full
string; update the block that computes service_pack (variables `last`, `sp`, and
the call to OsString::from_wide) to handle empty cases safely and use
`&version.szCSDVersion[..last + 1]` instead of `&version.szCSDVersion[..last]`.

In `@crates/host_env/src/wmi.rs`:
- Around line 660-661: The code calls CloseHandle(init_event) and
CloseHandle(connect_event) unconditionally; change it to only call CloseHandle
when the corresponding event handle was successfully created by CreateEventW
(i.e., check that init_event and connect_event are not null/INVALID_HANDLE_VALUE
before calling CloseHandle). Update the cleanup logic around the CreateEventW
calls and any early-return paths so each handle is closed only if it was
actually allocated, referencing the symbols CreateEventW, init_event,
connect_event, and CloseHandle to locate the relevant code.

---

Duplicate comments:
In `@crates/host_env/src/syslog.rs`:
- Around line 46-49: The TOCTOU bug occurs because closelog() calls is_open()
before acquiring the global_ident() write lock, allowing a concurrent openlog()
to race; modify closelog() to acquire the write lock on global_ident() first
(e.g., let mut locked_ident = global_ident().write()), then check whether the
saved identifier is Some via locked_ident.as_ref() or is_open() logic under that
lock, call unsafe { libc::closelog() } and set *locked_ident = None while still
holding the lock so close and state update happen atomically with respect to
openlog().

In `@crates/host_env/src/time.rs`:
- Around line 264-267: get_tz_info currently ignores the return code of
GetTimeZoneInformation and decodes a zeroed struct on failure; change
get_tz_info to check the return value against TIME_ZONE_ID_INVALID, call
GetLastError() when invalid, and return a Result<WindowsTimeZoneInfo,
std::io::Error> (or another appropriate error type) instead of unconditionally
returning WindowsTimeZoneInfo; update callers of get_tz_info to handle the
Result and propagate or map the error, and ensure the WindowsTimeZoneInfo
construction only happens on a successful GetTimeZoneInformation return.
- Around line 376-384: asctime_from_tm currently only guards against negative
weekday values but still indexes WDAY_NAME with tm.tm_wday (and similarly
MON_NAME with tm.tm_mon) which can panic for tm.tm_wday == 7 or tm.tm_mon > 11;
update asctime_from_tm to validate both tm.tm_wday is within 0..=6 and tm.tm_mon
is within 0..=11 before using WDAY_NAME[tm.tm_wday as usize] and
MON_NAME[tm.tm_mon as usize], and handle out-of-range values the same way the
function handles negative values (reject/return the existing fallback) to
prevent panics.

---

Nitpick comments:
In `@crates/host_env/src/cert_store.rs`:
- Around line 90-124: The enum_crls function only opens a single system store
(defaulting to CurrentUser) causing CRLs from LocalMachine to be missed; modify
enum_crls to mirror enum_certificates by iterating both store locations
(CurrentUser and LocalMachine) when opening the store (either call
CertOpenSystemStoreW twice with the two system store flags or use CertOpenStore
with CERT_SYSTEM_STORE_CURRENT_USER and CERT_SYSTEM_STORE_LOCAL_MACHINE),
enumerate CRLs with CertEnumCRLsInStore for each opened store, push CrlEntry
items (der and encoding_type(crl.dwCertEncodingType)) into the same result Vec,
and ensure each opened store is closed with CertCloseStore; keep using the same
types/functions (enum_crls, CertEnumCRLsInStore, CertCloseStore, CrlEntry,
encoding_type) to locate and update the code.

In `@crates/host_env/src/faulthandler.rs`:
- Around line 163-164: In current_thread_id(), replace the explicit cast "as
u64" on libc::pthread_self() with a generic cast "as _" to avoid tripping
clippy::trivial_numeric_casts while preserving portability (keep the unsafe {
libc::pthread_self() as _ } expression and function signature returning u64);
this mirrors the pattern used elsewhere (e.g., RustPython's _thread) and avoids
introducing a lint suppression.

In `@crates/host_env/src/io.rs`:
- Line 1: The import CStr and the function/variable fd_is_own are only used on
specific targets and should be cfg-gated to avoid cross-target unused-item
lints; wrap use core::ffi::CStr with #[cfg(any(unix, target_os = "wasi"))] (or
the exact cfg used elsewhere) and add #[cfg(not(windows))] or #[cfg(any(unix,
target_os = "wasi"))] to the fd_is_own definition (and similarly guard the code
at the 174-178 region) so the symbols are only compiled on targets that need
them, keeping function names like fd_is_own and usages of CStr unchanged.

In `@crates/host_env/src/mmap.rs`:
- Around line 91-94: The unsafe impls for Send and Sync on NamedMmap lack
documented safety invariants; add a clear comment above the unsafe impls for
NamedMmap that states why sharing across threads is safe (e.g., the internal
Windows HANDLE is thread-safe, the underlying mapping is process-wide, any
mutable access is synchronized or the mapping is immutable/only exposes
atomic-safe access, and Drop does not race with concurrent use), reference the
fields used (the raw HANDLE and mapping pointer/len) and mention any required
caller guarantees; ensure the comment explains that these invariants must hold
for Send/Sync to be sound and update tests or reviewers if there are additional
assumptions about lifetime, ownership, or synchronization.
- Around line 274-284: The two identical functions map_anon should be
consolidated: replace the separate #[cfg(unix)] and #[cfg(windows)] blocks with
a single #[cfg(any(unix, windows))] attribute on one map_anon implementation
that uses MmapOptions::new(), sets len(size), calls map_anon(), and maps to
MappedFile::Write; ensure the function signature pub fn map_anon(size: usize) ->
io::Result<MappedFile> and the internal calls to MmapOptions, map_anon, and
MappedFile::Write remain unchanged.

In `@crates/host_env/src/multiprocessing.rs`:
- Around line 420-434: Replace the conditional debug-only check with a hard
assert so passing a deadline on Apple fails loudly: change the
debug_assert!(deadline.is_none()) in the Apple-specific branch of the code
handling sem_wait/sem_timedwait to assert!(deadline.is_none()) so the invariant
is enforced in release builds; ensure you update the branch that calls unsafe {
libc::sem_wait(handle) } and returns WaitStatus::Acquired /
WaitStatus::Interrupted / WaitStatus::Error(SemError::from_errno(err))
accordingly.
- Around line 199-209: current_thread_id has platform-dependent return types
(unix: u64 via pthread_self, windows: u32 via GetCurrentThreadId) causing an
inconsistent API; change the Windows implementation of current_thread_id to
return u64 as well by casting GetCurrentThreadId() to u64 so both cfg(unix) and
cfg(windows) expose the same u64 signature, and update the function signature
for the Windows variant (and any callers if needed) to match current_thread_id
-> u64 while keeping the unix implementation using pthread_self() as before.

In `@crates/host_env/src/nt.rs`:
- Around line 1082-1127: The two functions path_exists_via_open and
test_file_exists_by_name duplicate CreateFileW/opening logic and the regular
reparse point retry; extract a small helper (e.g., open_check_exists_or_reopen)
that accepts the wide path, initial flags (FILE_FLAG_BACKUP_SEMANTICS | optional
FILE_FLAG_OPEN_REPARSE_POINT), desired access (FILE_READ_ATTRIBUTES), and a
boolean indicating whether to follow links, performs the CreateFileW call, runs
test_file_type_by_handle(handle, TestType::RegularReparsePoint, false), ensures
CloseHandle is always called, and returns true/false indicating existence
(including the retry open with FILE_FLAG_BACKUP_SEMANTICS if a regular reparse
point was observed). Replace the duplicated blocks in path_exists_via_open and
test_file_exists_by_name with calls to this helper.

In `@crates/host_env/src/os.rs`:
- Around line 118-126: The cfg attributes on the two cpu_count functions are
using a confusing double-negation; update the second attribute to the clearer
equivalent by replacing #[cfg(not(any(not(target_arch = "wasm32"), target_os =
"wasi")))] with #[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))] so
cpu_count() implementations read as "non-wasm32 or wasi" (the existing first fn)
and "wasm32 and not wasi" (the second fn) respectively; leave the function names
and bodies unchanged.

In `@crates/host_env/src/overlapped.rs`:
- Around line 74-75: The unsafe impls for Sync and Send on Operation are missing
a SAFETY justification even though Operation contains mutable fields (pending,
completed, read_buffer, write_buffer) that are mutated without internal
synchronization; add a clear `// SAFETY:` comment above `unsafe impl Sync for
Operation {}` and `unsafe impl Send for Operation {}` that describes why sharing
across threads is sound (e.g., external synchronization guarantees,
atomic/lock-free usage, or that only immutable methods are called concurrently),
or alternatively document on the `Operation` type that callers must ensure
exclusive access when invoking mutating methods like those that touch `pending`,
`completed`, `read_buffer`, and `write_buffer`. Include references to the
`Operation` struct and the specific fields/methods so future reviewers can
verify the invariants.
- Around line 1084-1116: The format_message function in overlapped.rs duplicates
windows.rs::format_error_message; remove the duplicated implementation and
replace callers of overlapped::format_message with a call to
windows::format_error_message(error_code) (or move the shared logic into a new
common helper and have both modules call that), ensuring you update imports to
bring windows::format_error_message into scope and delete the
LocalFree/FormatMessageW-specific code in overlapped.rs so only the single
canonical implementation remains.

In `@crates/host_env/src/posix.rs`:
- Around line 401-403: The pipe2 function currently returns nix::Result but
other functions use std::io::Result; change pub fn pipe2(...) ->
nix::Result<...> to pub fn pipe2(...) -> std::io::Result<(std::os::fd::OwnedFd,
std::os::fd::OwnedFd)> and convert the nix error to std::io::Error (e.g., via
.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))) on the
nix::unistd::pipe2(...) call so callers get a consistent std::io::Result from
pipe2.
- Around line 1147-1154: Remove the unnecessary rewinddir call in the Drop
implementation for FdDirStream: in the impl Drop for FdDirStream (the drop
method that currently calls libc::rewinddir and libc::closedir), delete the
libc::rewinddir(self.0.as_ptr()) call and keep only the
libc::closedir(self.0.as_ptr()) inside the unsafe block so the directory stream
is closed without resetting its position first.
- Around line 1564-1569: The helper fn should_keep currently returns true when
an fd should be closed (fd > above && not found in keep), which is
misleading—rename the function to something like should_close or is_closable
(e.g., fn should_close(above: i32, keep: &[BorrowedFd<'_>], fd: i32) -> bool)
and update all call sites that reference should_keep to the new name (the places
where it filters/decides which FDs to close). Keep the implementation identical
but use the new, semantically-correct identifier to avoid confusion.
- Around line 766-768: Change the chdir function to return std::io::Result<()>
instead of nix::Result<()>, and map any nix::Error into an std::io::Error before
returning; specifically, update the signature of chdir and replace the direct
nix::unistd::chdir(cwd) return with something like
nix::unistd::chdir(cwd).map_err(|e|
std::io::Error::new(std::io::ErrorKind::Other, e)) so the module API remains
consistent while preserving the original error information (referencing the
chdir function and nix::unistd::chdir call).
- Around line 801-841: Change the listed functions to return std::io::Result<()>
instead of nix::Result<()>, and convert any nix/libc errors to std::io::Error
before returning; specifically update signatures for setsid_if_needed,
setpgid_if_needed, setgroups_if_needed, setregid_if_needed and
setreuid_if_needed to -> std::io::Result<()>, then map/convert errors from calls
like nix::unistd::setsid(), nix::unistd::setpgid(...),
nix::unistd::setgroups(...), and nix::Error::result(ret) into std::io::Error
(e.g. using io::Error::from_raw_os_error from the errno or io::Error::new with
the nix error message) so the public API is consistent with the rest of the
module.

In `@crates/host_env/src/pwd.rs`:
- Around line 38-40: getpwnam currently swallows errors via .ok().flatten(),
making it inconsistent with getpwuid; change getpwnam signature to return
io::Result<Option<Passwd>> and propagate errors from User::from_name instead of
dropping them. Replace the .ok().flatten() pattern with proper error handling
(e.g., map the Result from User::from_name to io::Result and then map/transpose
into Option<Passwd>) so callers receive Err on underlying failures while still
returning Ok(None) when the user is absent; reference the getpwnam function and
User::from_name and match the error behavior of getpwuid.

In `@crates/host_env/src/select.rs`:
- Around line 183-209: The helper functions search_poll_fd, insert_poll_fd,
get_poll_fd_mut, and remove_poll_fd rely on the vector being sorted by PollFd.fd
but that invariant is implicit; either add a short rustdoc on each function (or
on the module) stating "fds must be sorted by fd" and the consequences, or
better encapsulate the Vec<PollFd> in a small wrapper type (e.g. PollFdList)
that owns the Vec and exposes methods search, insert, get_mut, remove which
maintain/validate the sorted-by-fd invariant internally; update these functions
to be methods on that wrapper (or call its validation) so accidental misuse is
prevented.

In `@crates/host_env/src/signal.rs`:
- Line 336: Replace the magic number 21 used in the signal mapping with the
SIGBREAK constant to improve consistency: locate the mapping or match arm that
currently has the entry `21 => "Break"` and change it to use the previously
defined SIGBREAK symbol (defined on line ~216) so it reads `SIGBREAK =>
"Break"`, ensuring imports/visibility of SIGBREAK are respected.
- Around line 176-192: The syscall result in pidfd_send_signal unnecessarily
casts the libc::syscall return to libc::c_long; remove the redundant cast so the
unsafe block assigns the syscall(...) directly to ret (which is declared as
libc::c_long), updating the assignment in the pidfd_send_signal function to
return the syscall result without the "as libc::c_long" cast.
- Around line 71-72: The call to siginterrupt(signalnum, 1) currently discards
its Result with let _ = ...; add a brief inline comment next to that statement
explaining why errors are intentionally ignored (for example: non-critical
best-effort to set SA_RESTART behavior, older platforms where siginterrupt may
be unavailable, or errors are expected and safe to ignore) so future readers
know this is deliberate; reference the siginterrupt call and the signalnum
variable when adding the comment.
- Around line 204-227: VALID_SIGNALS currently embeds the literal 21 instead of
the named constant SIGBREAK because SIGBREAK is defined below; move the SIGBREAK
declaration above VALID_SIGNALS (or define SIGBREAK before VALID_SIGNALS) and
replace the literal 21 in VALID_SIGNALS with SIGBREAK so the signal list
consistently uses the named constant (refer to VALID_SIGNALS and SIGBREAK to
locate and update the definitions).
- Around line 230-235: The init_winsock function currently requests WinSock 1.1
(WSAStartup called with 0x0101); update the requested version to WinSock 2.2 by
passing 0x0202 (or using MAKEWORD(2,2)) to the WSAStartup call in the
init_winsock block (symbols: init_winsock, WSA_INIT, WSAStartup, wsa_data) so
the code initializes WinSock 2.2 instead of 1.1.
- Around line 35-45: Remove the platform-specific ffi module that declares
unsafe extern "C" getitimer/setitimer and instead use the libc::getitimer and
libc::setitimer symbols uniformly on all Unix targets; locate the mod ffi block
and replace its usage sites (calls referencing ffi::getitimer / ffi::setitimer)
to call libc::getitimer and libc::setitimer directly, ensuring any unsafe blocks
and argument types match the existing calls that already use libc on other Unix
branches (see usages around getitimer and setitimer in this file).

In `@crates/host_env/src/socket.rs`:
- Around line 150-160: The cfg attribute on the socket-related code currently
redundantly lists many Unix target_os values alongside `unix`; replace the long
predicate with a single, maintainable condition—either #[cfg(unix)] or, if you
intend to exclude redox, use #[cfg(all(unix, not(target_os = "redox")))]—by
updating the #[cfg(...)] on the block in this file (the attribute immediately
above the socket-related definitions) to the chosen simplified form.
- Around line 194-206: The functions gai_error_string and h_error_string may
call CStr::from_ptr on a null pointer if libc::gai_strerror or libc::hstrerror
return null; change each function to call the libc function into a local pointer
(e.g., let p = libc::gai_strerror(err)), check p for null (std::ptr::null())
inside the unsafe block, and if null return a safe fallback string like
format!("Unknown gai error: {}", err) (and similarly for h_error_string:
"Unknown herror: {}"); otherwise construct the CStr and return its
to_string_lossy().into_owned() as before.

In `@crates/host_env/src/thread.rs`:
- Around line 4-6: The current_thread_id function casts libc::pthread_self()
with as u64 which triggers Clippy's trivial_numeric_casts on Linux; update the
cast to use as _ so the compiler infers the return type from the function
signature (i.e., change the unsafe block that returns libc::pthread_self() as
u64 to returning libc::pthread_self() as _), keeping the function name
current_thread_id and the libc::pthread_self reference intact.

In `@crates/host_env/src/time.rs`:
- Around line 22-27: The two cfg-annotated definitions of TimeT are identical
and should be collapsed into a single unconditional type alias (remove
#[cfg(unix)]/#[cfg(windows)] around TimeT); do the same for the identical cfg
branches that implement current_time_t and the other duplicated block around
lines ~73-77 by extracting any platform-specific values only and invoking the
shared logic once (reference symbols: TimeT and current_time_t) so duplicate
byte-for-byte branches are eliminated.

In `@crates/host_env/src/winapi.rs`:
- Around line 578-579: Add a SAFETY comment above the unsafe impls for BatchData
explaining why implementing Send and Sync is sound: note that BatchData contains
raw handles and an UnsafeCell, and document the required invariants (e.g., all
mutable writes to BatchData occur-before the thread is resumed, and all reads
occur-after the thread completes/joins; no concurrent access occurs; ownership
of raw handles is not aliased across threads). Reference the symbols BatchData
and the unsafe impl Send/Sync to make clear which types and impls the comment
applies to.

In `@crates/host_env/src/winreg.rs`:
- Around line 89-96: The function bytes_as_wide_slice relies on a debug-only
assert and must be made explicit about alignment guarantees; change
bytes_as_wide_slice to an unsafe fn bytes_as_wide_slice(bytes: &[u8]) -> &[u16]
and add a doc comment stating callers must guarantee the input is u16-aligned
and valid UTF-16/registry data, keep or tighten the existing
debug_assert!(prefix.is_empty() && suffix.is_empty(), ...) for debug builds, and
update all call sites to call it within an unsafe block so alignment
responsibility is clearly documented and enforced at the call boundary.
- Around line 421-425: The code handling the initial query_value_ex result masks
real errors by forcing buf_size = 256 when buf_size == 0 even if the call
returned an error other than Foundation::ERROR_MORE_DATA; update the logic in
the query loop (the block that inspects res and buf_size) to preserve and check
the original res: only treat ERROR_MORE_DATA (or a zero-size success) as a
resize-and-retry case, but if res != 0 && res != Foundation::ERROR_MORE_DATA
then return Err(res) immediately (or return the preserved original error)
instead of overwriting buf_size; ensure the condition references the same res
from query_value_ex and that query_value_ex, buf_size, and
Foundation::ERROR_MORE_DATA are the symbols used to locate the change.

In `@crates/host_env/src/wmi.rs`:
- Around line 670-675: The code can underflow when offset is odd (e.g., offset
== 1) because (offset as usize) / 2 - 1 can wrap; update the calculation to be
defensive: compute let bytes = offset as usize, then guard bytes < 2 => return
Ok(String::new()), or use let char_count = bytes / 2 .saturating_sub(1) (or
adjust bytes = bytes.saturating_sub(bytes % 2)) before indexing buffer; apply
this fix where offset, buffer and char_count are computed so you never perform a
subtraction that can underflow.

In `@crates/stdlib/src/mmap.rs`:
- Around line 406-437: The AccessMode-to-host_mmap::AccessMode mapping is
duplicated in multiple places (e.g., the closure that calls host_mmap::map_file
in the mmap constructor and other locations), so add a single conversion method
or impl to centralize it; implement either an impl AccessMode { fn
to_host(&self) -> host_mmap::AccessMode } or impl From<AccessMode> for
host_mmap::AccessMode, then replace the repeated match blocks wherever
AccessMode is converted (references: AccessMode, host_mmap::AccessMode,
host_mmap::map_file) to call the new conversion method or From::from to avoid
repetition.

In `@crates/stdlib/src/openssl.rs`:
- Around line 4107-4137: The code in enum_certificates currently ignores
CertificateEntries.had_open_store returned by
rustpython_host_env::cert_store::enum_certificates and silently returns an empty
list; update enum_certificates to check the returned
CertificateEntries.had_open_store and, if false, raise an OSError via the VM
(e.g. return Err(vm.new_os_error("could not open certificate store")) or
similar) before mapping entries; reference the enum_certificates function, the
CertificateEntries.had_open_store field, and
rustpython_host_env::cert_store::enum_certificates when making the change.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/host_env/src/crt_fd_unsupported.rs`:
- Around line 65-71: The try_from_raw constructor currently only treats -1 as
invalid; change its validation to reject any negative Raw value (fd < 0) and
return ebadf() instead of constructing the wrapper; apply the same fix to the
other try_* constructor(s) in this file (see the other
try_from_raw/try_from_raw-like function around the 121-127 region) so all
negative file descriptors (e.g., -2, -3) produce Err(ebadf()) rather than
Ok(...).

In `@crates/host_env/src/io_unsupported.rs`:
- Around line 84-126: In parse_fileio_mode the branch handling the binary
modifier (b'b' => {}) currently allows it to appear multiple times (e.g.,
"rbb"); add a boolean flag (e.g., b_seen) initialized false and update the b
branch to return Err(FileModeError::Invalid) if b_seen is already true,
otherwise set b_seen = true (so the loop rejects duplicate 'b' modifiers);
reference the parse_fileio_mode function and the b'b' match arm when making this
change.

In `@crates/host_env/src/posix_wasi.rs`:
- Around line 32-68: stat_path currently treats a CString::new(...) failure
(embedded NUL) as Ok(None); change it to return an error instead by converting
the CString error into an io::Error with InvalidInput (or EINVAL) so callers can
distinguish interior-NUL input from "no result"—update the early match in
stat_path (the CString::new(...) branch) to return
Err(io::Error::from(io::ErrorKind::InvalidInput)) and make the same change in
the duplicate implementation in posix.rs (the stat_path there) for consistency.

In `@crates/host_env/src/posix.rs`:
- Around line 1655-1672: close_fds_brute_force assumes keep is in ascending
order (and should_keep uses binary_search_by_key), so if keep is unsorted some
fds will not be closed; either enforce the precondition at the public entry
(close_fds) by sorting the slice (or collecting and sorting by
BorrowedFd::as_raw_fd) before passing to should_keep/close_fds_brute_force, or
add a debug-assert in close_fds that verifies keep is sorted ascending (compare
successive as_raw_fd values) and document the requirement in close_fds'
docstring; update callers if you change the API contract.

In `@crates/stdlib/src/mmap.rs`:
- Around line 468-484: The code incorrectly treats fileno == 0 as anonymous;
change the branch that computes fh so only fileno == -1 is considered anonymous
(i.e. remove the fileno != 0 check) so that fileno 0 goes through
host_nt::handle_from_fd and the subsequent host_mmap::is_invalid_handle_value
check; update the surrounding comment to reflect that only -1 signals anonymous
mapping and keep using host_nt::handle_from_fd,
host_mmap::is_invalid_handle_value and vm.new_os_error as before.

---

Duplicate comments:
In `@crates/host_env/src/ctypes.rs`:
- Around line 1926-1938: In copy_com_pointer, the vtable is only checked with
debug_assert! which is skipped in release builds; add a real runtime null-check
on the vtable (the value loaded into `vtable` from `iunknown`) before
dereferencing or transmuting it, and return an appropriate HRESULT (e.g.
HRESULT_E_POINTER) if the vtable is null or invalid; ensure the check happens
immediately after computing `iunknown`/`vtable` and before the transmute to
`addref_fn` so the unsafe block never dereferences a null/bad vtable in release.
- Around line 760-815: In read_array_element ensure we guard offset before any
buffer[offset..] slice to avoid panics: add an early check (e.g. if offset >=
buffer.len() return DecodedValue::None) or replace raw slices with safe
indexed/get calls where used in the Some("u"), Some("f"), Some("d"|"g") branches
and in the default path before calling buffer[offset..].first_chunk or
get(..element_size); update the branches using
wchar_from_bytes(&buffer[offset..]), first_chunk::<4/8>(), and
buffer[offset..].get(..element_size) to use the guarded/safe slice so invalid
Python indices hit the existing fallback instead of panicking the VM.
- Around line 201-205: The code defines LONG_DOUBLE_SIZE as a wider type but the
encoder/decoder and libffi mapping still treat the format code "g" as f64;
update the marshal logic so "g" maps to a proper C long double representation:
modify the encoder/decoder functions that handle format code "g" to branch based
on LONG_DOUBLE_SIZE and target ABI (x86_64/aarch64 non-Windows) and use the Rust
libffi crate's long-double Type when available (or a size-specific raw buffer
fallback), ensuring the libffi type mapping, ffi call setup, and
serializer/deserializer all use the same size/representation (80-bit x86
extended, 128-bit on platforms that use 16-byte long double) instead of always
using f64; adjust any code that references the "g" case (encoder/decoder
handlers and the libffi mapping table) to keep ABI size/representation
consistent with LONG_DOUBLE_SIZE and add unit tests for both x86_64 and aarch64
branches.
- Around line 428-498: All helpers treat Windows wchar_t as full Unicode code
points instead of UTF‑16 code units, which corrupts non‑BMP characters; update
wstring_from_bytes, wchar_array_field_value, write_wchar_array_value,
encode_wtf8_to_wchar_padded, and wchar_null_terminated_bytes to operate on u16
code units (WCHAR) not u32 code points: read/write chunks via
u16::from_le_bytes/into_le_bytes (respecting WCHAR_SIZE), decode sequences using
core::char::decode_utf16 (or std::char::decode_utf16) to correctly combine
surrogates into Rust chars, and when encoding use Iterator::encode_utf16 on the
Rust string or Wtf8 code points to produce u16 units before serializing to
bytes; also ensure buffer length checks use u16 counts and write a u16 zero
terminator when space permits.
- Around line 2431-2464: The current get_pointer uses core::mem::transmute_copy
on libloading::Library which relies on private layout; instead, add a dedicated
raw handle field to SharedLibrary (e.g., raw_handle: Mutex<Option<usize>> or
platform-specific handle type) and set that field when constructing the
SharedLibrary in new, new_with_mode and from_raw_handle (use
UnixLibrary::into_raw/Library::into_raw on unix/windows or convert the
os-specific Library into a raw handle there), then have get_pointer return the
stored raw_handle (under lock) rather than transmuting the Library; update
constructors to extract and store the raw handle and avoid transmute_copy
entirely.
- Around line 134-147: The helpers leak the foreign-call OS error back into the
host by not restoring the original OS slot; in with_swapped_errno (and the
analogous with_swapped_last_error helper) capture the post-call OS error and
store it into CTYPES_LOCAL_ERRNO (or the ctypes-local LastError), then restore
the saved host OS error slot (call crate::os::set_errno(saved) / the equivalent
set_last_error with the previously saved value) before returning so the
embedding thread's OS error state is preserved.
- Around line 1325-1341: The match arms for type codes "l"/"L" currently write
fixed-width i64/u64 but must use platform C long widths; split the combined ("l"
| "q") and ("L" | "Q") arms so "l" maps to libc::c_long and "L" to libc::c_ulong
(cast the CallbackResultValue v to libc::c_long/ libc::c_ulong and write to
result as *mut libc::c_long/*c_ulong), while keeping "q"/"Q" mapped to i64/u64
as before; update the match in ctypes.rs (the arms handling type_code
"l","L","q","Q" and CallbackResultValue) so FFI memory access uses
libc::c_long/c_ulong for 'l'/'L' and fixed-width i64/u64 for 'q'/'Q'.

In `@crates/host_env/src/posix.rs`:
- Around line 458-469: The utimes function's timeval builder closure tv uses
d.as_micros() for tv_usec which gives total microseconds instead of the
fractional microsecond part, causing invalid tv_usec values; change tv to use
d.subsec_micros() (or compute d.as_micros() % 1_000_000) for tv_usec so tv_sec
is d.as_secs() and tv_usec is the sub-second microsecond portion, matching the
convention used by set_file_times_at and preventing EINVAL from utimes.

In `@crates/host_env/src/select.rs`:
- Around line 96-105: In FD_SET, validate capacity before mutating __nfds:
compute n = set.__nfds, check that n < FD_SETSIZE (and/or that set.__fds has
room) before writing to set.__fds[n] and incrementing set.__nfds; if the set is
full return early (or handle overflow) so you never advance __nfds and then
index out of bounds. Reference: function FD_SET, fields __nfds and __fds, and
constant FD_SETSIZE.
- Around line 287-295: The epoll wait writes into the Vec's spare capacity but
never updates events.len(), so callers see an empty vector; in the wait
function, after calling rustix::event::epoll::wait and receiving Ok(n), set the
Vec length to n (e.g. unsafe { events.set_len(n); }) before returning Ok(n) so
the written entries are published (refer to the wait function, the events Vec,
and rustix::event::epoll::wait/spare_capacity usage).

In `@crates/host_env/src/syslog.rs`:
- Around line 41-50: The race happens because libc::syslog() is called without
holding the ident read lock while closelog() can free the ident; fix by holding
the global_ident() RwLock read guard across the libc::syslog() call (i.e.
acquire read lock in syslog() and use the held reference/pointer for the call so
the ident cannot be dropped concurrently), and change closelog() to acquire the
write lock unconditionally and clear/closelog() under that write lock (remove
the pre-check via is_open() or make it check under the write lock) so the write
lock serializes with syslog()’s read lock; reference functions: syslog,
closelog, is_open, global_ident.

---

Nitpick comments:
In `@crates/host_env/src/io.rs`:
- Around line 215-221: Replace the OS errno checks with ErrorKind checks for
symmetry and portability: update is_interrupted_error to return err.kind() ==
io::ErrorKind::Interrupted (matching the would-block pattern used by
is_would_block_error) and ensure is_would_block_error uses err.kind() ==
io::ErrorKind::WouldBlock so both functions use std::io::ErrorKind rather than
raw_os_error()/libc constants.

In `@crates/host_env/src/posix_wasi.rs`:
- Around line 5-103: The posix_wasi.rs file duplicates implementations found in
posix.rs (make_dir, make_dir_at, remove_dir_at, stat_path, stat_fd,
set_file_times_at); remove posix_wasi.rs and consolidate by adjusting cfg
attributes in posix.rs so WASI builds use the shared implementations (e.g.,
expand #[cfg(not(windows))] / #[cfg(all(any(target_os = "wasi", unix),
not(target_os = "redox")))] to include target_os = "wasi" where needed), update
lib.rs so it refers to the single posix module, and run a build/test to verify
there are no cfg conflicts; alternatively, if any WASI-specific divergence is
required, extract common helpers (error-from-neg, timespec builder,
MaybeUninit<libc::stat> handling) into a small private helper module and have
both posix.rs and posix_wasi.rs call those helpers instead.

In `@crates/host_env/src/posix.rs`:
- Line 705: Replace the #[allow(clippy::too_many_arguments)] suppression by
introducing a small struct (e.g. ChildFds { p2c_read, p2c_write, c2p_read,
c2p_write, err_read, err_write, errpipe_read }) to encapsulate the six i32 fd
parameters and related values; update the functions that currently take nine
arguments (the one guarded by the attribute and any helpers that pass those fds)
to accept a ChildFds instance (and any remaining non-fd params) and change all
call sites to construct ChildFds with named fields, removing the lint
suppression and eliminating positional fd confusion.
- Around line 723-728: The loop that closes p2cwrite, c2pread, errread and the
subsequent close(errpipe_read) should attempt to close every fd and only return
the first encountered error instead of bailing out immediately; in
setup_child_fds, replace the current direct map_err? usage with logic that
iterates the fds [p2cwrite, c2pread, errread, errpipe_read], for each fd >= 0
call nix::unistd::close(fd) and if it returns Err capture the first
std::io::Error (via std::io::Error::from) but continue closing the rest, and
after all closes return Ok(()) if no error was captured or Err(first_error) if
one was.
- Around line 770-829: The listed functions (chdir, setsid_if_needed,
setpgid_if_needed, setgroups_if_needed and the similar
setregid_if_needed/setreuid_if_needed) expose nix::Result/nix::Error; change
their public signatures to return std::io::Result<(),> and map any nix errors
into std::io::Error before returning. Implement mapping like: on nix Err -> if
e.as_errno().is_some() use std::io::Error::from_raw_os_error(errno as i32) else
use std::io::Error::new(io::ErrorKind::Other, e.to_string()); keep the existing
logic and use ?-style by replacing `?` with `.map_err(|e| /*map as
described*/)?` so callers get std::io::Result consistently across host_env.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/host_env/src/cert_store.rs`:
- Around line 101-121: When CertEnumCRLsInStore(store, crl_context) returns NULL
you must call GetLastError() to distinguish normal end-of-enumeration from an
actual error; update the loop/exit logic around crl_context so that after
breaking on a NULL you call GetLastError() and if it returns ERROR_SUCCESS (0)
continue to return Ok(result), otherwise propagate an Err (convert the Win32
error code into your crate's error type). Ensure CertCloseStore(store, 0) still
runs in both success and error paths (use a single cleanup block or early-return
pattern that closes the store before returning the Err). Reference:
CertEnumCRLsInStore, crl_context, CertCloseStore, GetLastError, ERROR_SUCCESS.
- Around line 41-47: The encoding_type function currently does exact-match
matching and must instead test dwCertEncodingType bitflags: change fn
encoding_type(raw: u32) -> EncodingType to use bitwise checks (e.g., if raw &
X509_ASN_ENCODING != 0 { return EncodingType::X509Asn } else if raw &
PKCS_7_ASN_ENCODING != 0 { return EncodingType::Pkcs7Asn } else {
EncodingType::Other(raw) }), so combined flags like X509_ASN_ENCODING |
PKCS_7_ASN_ENCODING are correctly classified (prefer X509Asn when both are
present, or adjust preference if you have a different rule).

In `@crates/host_env/src/faulthandler.rs`:
- Around line 396-411: The Windows path in enable_fatal_handlers currently
returns false on the first install_signal_handler failure but leaves earlier
entries marked enabled (leaking partial installs); modify enable_fatal_handlers
to mirror the Unix rollback: keep a local Vec (imported with #[cfg(any(unix,
windows))]) of indices or entries that were successfully installed, and if an
install_signal_handler call returns Err, iterate the recorded installs and call
disable_fatal_signal (or otherwise restore entry.previous and set entry.enabled
= false) for each to revert them, then return false; ensure you update each
FATAL_SIGNAL_HANDLERS entry's previous/ enabled fields only after a successful
install and clear them during rollback.
- Line 90: USER_SIGNAL_CAPACITY is set to 64 which excludes SIGRTMAX (typically
64) — bump USER_SIGNAL_CAPACITY to 65 so the valid signum space includes 64,
update any signum-range validations such as the C-level check (the guard using
`signum as usize >= USER_SIGNAL_CAPACITY`) and corresponding Python-level ranges
(replace exclusive upper bounds like `1..64` with inclusive ranges or
`1..=64`/adjusted checks) so the highest real-time signal can be registered.

In `@crates/host_env/src/locale.rs`:
- Around line 25-30: Remove the six unused trailing fields int_p_cs_precedes,
int_p_sep_by_space, int_n_cs_precedes, int_n_sep_by_space, int_p_sign_posn,
int_n_sign_posn from the RawLconv struct to match the actual runtime lconv
layout (ensure localeconv_data and LocaleConv use only fields up through
n_sign_posn); update strxfrm so it returns a Vec<u8> that does NOT include the
trailing NUL byte (or change the caller in stdlib's strxfrm usage so it strips
the final NUL before calling String::from_utf8), and change decode_ansi_bytes so
that an empty input returns Some(String::new()) instead of None.
- Around line 116-124: The strxfrm function currently allocates twice and
returns a buffer with a trailing NUL; change it to query required size once by
calling libc::strxfrm with a null dest (size 0) to get n, allocate a single
buffer of n+1 bytes, call libc::strxfrm once to fill it, then return a Vec<u8>
truncated to n (omitting the terminating NUL) so consumers (e.g., the code using
String::from_utf8(buff)) receive no embedded NUL; update the function strxfrm to
perform the null-query, single allocation, one fill call, and return only the n
bytes.

In `@crates/host_env/src/multiprocessing.rs`:
- Around line 128-131: The unlink=true branch currently calls unsafe {
libc::sem_unlink(cname.as_ptr()) } and ignores failures, leaking the name;
update this path in the function (the create/open routine that handles the
unlink flag) to check the return value of libc::sem_unlink, and on non‑zero
return convert errno into a Rust error (e.g., std::io::Error::from_raw_os_error
or nix::Error) and return Err instead of proceeding; reference the cname
variable and sem_unlink call so the error is propagated to callers rather than
returning success with a leaked name.
- Around line 383-394: In deadline_from_timeout, first validate the input:
reject non-finite or negative timeouts (use timeout.is_finite() and timeout >=
0.0) and return Err(SemError) for invalid values; then compute seconds and
nanoseconds with safe conversions and checked arithmetic instead of direct casts
— use timeout.floor() to get sec_f64 and convert to a libc time type with a
checked/saturating conversion, compute nsec = round((timeout - sec_f64) * 1e9)
into a libc integer, and use checked_add/checked_mul when combining tv.tv_usec *
1000 + nsec and when adding sec to tv.tv_sec; normalize tv_nsec (<
1_000_000_000) and if any checked operation overflows return Err(SemError).
Reference deadline_from_timeout, gettimeofday, tv_sec, tv_usec and tv_nsec when
applying these checks and replacements.
- Around line 88-96: Replace use of POSIX/Win32 last error for WinSock calls: in
paths handling closesocket, recv, and send, call WSAGetLastError() (from
windows_sys::Win32::Networking::WinSock) instead of
GetLastError()/last_os_error(); update any error logging/returns to use that
value. Also prevent truncating usize to i32 for socket buffer lengths in the
recv/send call sites (and any places that cast buffer lengths): check the usize
against i32::MAX and either clamp or return an appropriate error if it exceeds
i32::MAX, or use try_into() with explicit handling to avoid wrapping; look for
usages of closesocket, recv, send and the existing usize as i32 casts and fix
them accordingly. Ensure imports are updated to include WSAGetLastError and
remove reliance on INVALID GetLastError where appropriate.

In `@crates/host_env/src/overlapped.rs`:
- Around line 169-223: The Operation's completed flag must be cleared before
beginning a new async call; in connect_named_pipe, write, and read set
self.completed = false immediately before initiating the OS call (i.e. before
calling start_connect_named_pipe / start_write_file / start_read_file) so a
reused Operation won't treat a fresh I/O as already completed and truncate/zero
buffers prematurely; keep existing error handling and pending logic unchanged.
- Around line 54-61: The Drop impl currently frees Operation while kernel I/O
may still be pending; ensure Drop does not free overlapped/ buffers until the
kernel finishes: when scheduling read()/write()/connect_named_pipe() set
Operation.completed = false and Operation.pending = true on ERROR_IO_PENDING; in
Drop, if pending is true call CancelIoEx(self.handle,
self.overlapped.as_mut_ptr()) and then wait for completion (e.g.,
WaitForSingleObject on overlapped.hEvent or GetOverlappedResult with bWait =
TRUE) before closing the event handle and freeing memory so the kernel cannot
write into freed buffers; also reset completed = false whenever reusing
Operation for a new read/write so get_result(false) cannot return stale state.

In `@crates/host_env/src/testconsole.rs`:
- Around line 24-39: The loop calling WriteConsoleInputW in testconsole.rs can
spin if the call succeeds but writes zero events; after the unsafe call and
after checking res == 0, add a guard that if wrote == 0 you return
Err(io::Error::new(io::ErrorKind::WriteZero, "WriteConsoleInputW made no
progress")) so the function does not loop forever; update the block around the
WriteConsoleInputW call (the wrote and total variables and their handling) to
return this WriteZero error when no progress is observed.

In `@crates/host_env/src/thread.rs`:
- Around line 12-13: The current byte-slice truncation
(&c_name.as_bytes()[..15]) can cut a multi-byte UTF‑8 character; change to
truncate at the last valid UTF‑8 character boundary <=15 bytes: compute a byte
index end = 15 then while !c_name.is_char_boundary(end) { end -= 1 } (guarding
if end becomes 0) and use CString::new(&c_name[..end]).unwrap_or(c_name) to
build truncated; keep the same variable names (c_name, truncated) so the rest of
the code is unchanged.
- Around line 1-2: Replace the alloc import with use std::ffi::CString; and
adjust the thread-name truncation to avoid slicing in the middle of UTF-8
codepoints: convert the name to a String and truncate by characters (e.g.,
name.chars().take(15).collect::<String>()) before calling CString::new; then
remove the redundant unwrap_or(c_name) since CString::new on a null-free &str
cannot fail. Update references to the old import, the .as_bytes()[..15] slice,
and the CString::new(...).unwrap_or(c_name) call accordingly.

---

Duplicate comments:
In `@crates/host_env/src/ctypes.rs`:
- Around line 1973-1983: The code checks vtable nullity but still directly
dereferences *vtable.add(1) then transmutes it to addref_fn; instead validate
the AddRef slot before transmuting by reading let slot = *vtable.add(1) (or
equivalent), check if slot.is_null() and return HRESULT_E_POINTER if so, only
then transmute slot into the extern "system" fn and call it; refer to symbols
src_ptr, iunknown, vtable, addref_fn, and HRESULT_E_POINTER when making the
change.
- Around line 2510-2517: The get_pointer method is currently using unsafe
transmute_copy on libloading::Library (get_pointer) which relies on Library's
private layout; replace this by deriving a stable, supported cache key: either
convert the Library to a raw handle using the platform-specific supported API
(libloading::os::{unix, windows}::Library::into_raw and from_raw) or maintain a
separate stable ID (e.g., incrementing usize or UUID) stored alongside the
Library in the cache; update get_pointer to return that stable handle/ID (or the
platform raw handle obtained via the supported into_raw/from_raw/appropriate
accessor) instead of transmuting Library. Ensure you do not accidentally
move/consume the Library when getting the key (if using into_raw/from_raw,
reconstruct the Library with from_raw immediately) and update any cache
insert/remove logic that currently expects the transmuted value.
- Around line 205-221: The code advertises LONG_DOUBLE_SIZE as wider on Unix
x86_64/aarch64 but still treats format code "g" as f64 everywhere; update all
places that serialize/deserialize and map C types for the "g" (long double) ABI
to use LONG_DOUBLE_SIZE and the correct Rust/C type (c_longdouble or a
u128/bytes buffer matching LONG_DOUBLE_SIZE) and map to the libffi long-double
ffi type (or create a custom ffi::Type for long double) instead of f64;
specifically change the handling sites that reference LONG_DOUBLE_SIZE and the
format code "g" (serialization/deserialization routines and FFI type mapping
functions where "g" is currently sent to f64) so they use LONG_DOUBLE_SIZE, use
libc::c_longdouble or an appropriately sized byte representation, and select the
libffi long-double variant (or return an error if the libffi crate lacks
long-double support).

---

Nitpick comments:
In `@crates/host_env/src/thread.rs`:
- Around line 12-13: The fallback unwrap_or(c_name) is unreachable because
c_name was constructed via CString::new so its byte sub-slices cannot contain
nulls; update the truncation to use
CString::new(&c_name.as_bytes()[..15]).unwrap() (or .expect("...")) so the
invariant is explicit and dead code is removed—change the expression that
assigns truncated (using c_name and CString::new) to call unwrap()/expect()
instead of unwrap_or(...).

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/host_env/src/locale.rs`:
- Around line 144-166: The code casts bytes.len() to i32 which overflows for
inputs > i32::MAX; before calling MultiByteToWideChar, guard the input length by
checking if bytes.len() > i32::MAX as usize and return None (or an error) for
too-large slices, then compute a safe_i32_len = bytes.len() as i32 and use that
variable in both MultiByteToWideChar calls (replacing the inline casts). Do this
in the function that converts the &[u8] to wide (the block that declares len and
wide and calls MultiByteToWideChar) so the API never receives a negative
cbMultiByte.

In `@crates/host_env/src/multiprocessing.rs`:
- Around line 210-214: The Windows implementation of current_thread_id() returns
u32 while other targets and callers expect u64; change the function signature
pub fn current_thread_id() -> u64 and cast the unsafe GetCurrentThreadId()
result to u64 so the function consistently returns u64 across targets (update
the #[cfg(windows)] current_thread_id implementation that calls
GetCurrentThreadId()).

In `@crates/host_env/src/overlapped.rs`:
- Line 545: The public functions start_accept_ex, start_connect_ex,
start_disconnect_ex, and start_transmit_file currently call .get().unwrap() on
their OnceLock extension statics (e.g. ACCEPT_EX), which panics if
initialize_winsock_extensions() was not run; change each to safely handle a
missing initializer by replacing the unwrap with a checked .get() and returning
a proper Winsock error (e.g. WSAEOPNOTSUPP or another appropriate error code) or
attempting to call initialize_winsock_extensions() then rechecking, so the
functions fail via the normal error path instead of panicking; apply the same
pattern to all four functions and/or factor the logic into a small helper like
init_or_error! to centralize the check and error return.
- Around line 188-227: write() and read() replace their owned
write_buffer/read_buffer Vecs unconditionally which drops buffers while a kernel
async I/O may still be using their pointers (when pending == true), causing
use-after-free; fix by refusing to replace buffers or starting a new operation
when an I/O is pending: add a check at the start of overlapped::write and
overlapped::read that returns an Err (or io::ErrorKind::WouldBlock) if
self.pending is true, or alternatively provide and use a public
is_pending()/ensure_not_pending() helper so callers can check before calling;
update callers/tests accordingly and ensure the check is performed before
assigning to self.write_buffer/self.read_buffer and before calling
start_write_file/start_read_file to avoid dropping live buffers.
- Around line 855-908: The code leaks the Arc passed as data_ptr when a
registered wait is cancelled without the callback running; update the
WaitCallbackData struct to include an AtomicBool fired and also store the raw
pointer (data_ptr/raw_ptr) in the registry entry; in post_to_queue_callback set
data.fired.store(true, Ordering::Release) before consuming the Arc via
Arc::from_raw, and in cleanup_wait_callback_data after removing the registry
entry check if !entry.fired.load(Ordering::Acquire) and if so reclaim the leaked
raw Arc by calling unsafe { Arc::from_raw(entry.raw_ptr) } so the allocation is
dropped when the callback never fires.

In `@crates/host_env/src/thread.rs`:
- Around line 4-7: Add a Windows implementation of current_thread_id() and
normalize its public return type to u64 across platforms: keep the existing
#[cfg(unix)] pub fn current_thread_id() -> u64 as-is and add a #[cfg(windows)]
pub fn current_thread_id() -> u64 that calls the Windows API to obtain the
thread id (e.g., GetCurrentThreadId or equivalent) and casts the returned
DWORD/u32 to u64 before returning; ensure the symbol name is identical
(current_thread_id) so callers (including code in multiprocessing.rs that
currently uses u32 IDs) see a consistent u64 API.

---

Duplicate comments:
In `@crates/host_env/src/multiprocessing.rs`:
- Around line 393-404: Validate that the timeout is finite and non-negative in
deadline_from_timeout, then perform checked conversions and arithmetic instead
of unchecked casts: return an Err(SemError) for non-finite or negative inputs,
compute whole seconds and fractional nanoseconds using safe operations (e.g.,
timeout.floor() / fract or similar), use checked casts to libc::time_t and
libc::c_long for sec/nsec and checked_add for combining with gettimeofday()'s
tv_sec/tv_usec to detect overflow, normalize tv_nsec into [0,1_000_000_000)
using checked math, and return Err(SemError) on any overflow/overflow-prone
step; refer to deadline_from_timeout, gettimeofday, libc::timespec and SemError
when locating the code to update.

---

Nitpick comments:
In `@crates/host_env/src/overlapped.rs`:
- Around line 1079-1106: Change unparse_address to take a raw pointer
representing polymorphic sockaddr memory (e.g., *const SOCKADDR and an addr_len)
instead of &SOCKADDR_IN6, validate addr_len before dereferencing, and branch on
the family field to safely cast to SOCKADDR_IN or SOCKADDR_IN6; specifically,
update the signature of unparse_address to accept (*const SOCKADDR, i32) (or
*const u8 plus length), check that addr_len is at least the size needed for the
family and for the chosen target (sizeof(SOCKADDR_IN) for AF_INET,
sizeof(SOCKADDR_IN6) for AF_INET6) before performing unsafe casts, and then use
the existing logic that reads sin_family, casts to SOCKADDR_IN or SOCKADDR_IN6,
and constructs SocketAddress; keep the same error handling for unsupported
families and return an InvalidInput error when addr_len is too small.