GeezerVison

crates/stdlib/src/multiprocessing.rs (1)

805-822: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing cold_path() on the SemError::InteriorNul branches.

Every other NUL-error return site touched by this PR (grp.rs, openssl.rs::set_ciphers/new_py_ssl_socket, mmap.rs, nt.rs::execve, ssl.rs::validate_hostname) calls cold_path() immediately before returning the interior-NUL error, per the PR's stated goal of marking these exceptional paths cold. These two branches in py_new and sem_unlink return exceptions::nul_char_error(vm) on SemError::InteriorNul without a cold_path() call, leaving this file's semaphore error paths on the "hot" path unlike everywhere else.

♻️ Add cold_path() to match the cohort convention

+use core::hint::cold_path;
...
             SemHandle::create(&args.name, value, args.unlink).map_err(|err| {
                 if err == SemError::InteriorNul {
+                    cold_path();
                     exceptions::nul_char_error(vm)
                 } else {
                     os_error(vm, err)
                 }
             })?;
...
     fn sem_unlink(name: String, vm: &VirtualMachine) -> PyResult<()> {
         host_multiprocessing::sem_unlink(&name).map_err(|err| {
             if err == SemError::InteriorNul {
+                cold_path();
                 exceptions::nul_char_error(vm)
             } else {
                 os_error(vm, err)
             }
         })
     }

Also applies to: 838-847

🤖 Prompt for AI Agents

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

In `@crates/stdlib/src/multiprocessing.rs` around lines 805 - 822, Add cold_path()
immediately before returning exceptions::nul_char_error(vm) in both the
SemError::InteriorNul branch of py_new and the corresponding branch in
sem_unlink. Keep the existing non-NUL error handling unchanged and match the
cold-path convention used by the other touched NUL-error sites.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

crates/vm/src/function/fspath.rs (1)

43-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the shared NUL-error path.

The two arms differ only in the source value and constructed variant; derive the NUL flag and FsPath in the match, then perform the check once.

Proposed refactor

-                s @ PyStr => {
-                    if check_for_nul && s.contains_nuls() {
-                        cold_path();
-                        return Err(crate::exceptions::nul_char_error(vm));
-                    }
-                    Self::Str(s)
-                }
-                b @ PyBytes => {
-                    if check_for_nul && b.contains_nuls() {
-                        cold_path();
-                        return Err(crate::exceptions::nul_char_error(vm));
-                    }
-                    Self::Bytes(b)
-                }
+                s @ PyStr => (s.contains_nuls(), Self::Str(s)),
+                b @ PyBytes => (b.contains_nuls(), Self::Bytes(b)),

As per coding guidelines, “When branches differ only in a value but share common logic, extract the differing value first, then call the common logic once to avoid duplicate code.”

🤖 Prompt for AI Agents

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

In `@crates/vm/src/function/fspath.rs` around lines 43 - 53, Refactor the match
arms for PyStr and PyBytes to first derive the shared NUL-check flag and
corresponding FsPath value, then perform the check_for_nul/contains_nuls
validation once before returning the constructed value. Preserve the existing
cold_path and nul_char_error behavior, while retaining the distinct Str and
Bytes variants.

Source: Coding guidelines