◐ Shell
clean mode source ↗

Rewrite of winreg module and bump to 3.13.2 by arihant2math · Pull Request #5594 · RustPython/RustPython

arihant2math

6-8: Module wrapper delegation is correct

make_module simply forwards to the inner winreg::make_module(vm), which matches the usual pattern for #[pymodule]-based stdlib modules in this codebase. No changes needed.

45-59: Error helper and QueryValue behavior look consistent with CPython

  • os_error_from_windows_code correctly maps common Win32 codes (FILE_NOT_FOUND, ACCESS_DENIED) to more specific Python exception types and otherwise falls back to OSError, with a [WinError {code}]-style message. This aligns well with how CPython’s winreg surfaces OS errors. (learn.microsoft.com)
  • QueryValue:
    • Rejects HKEY_PERFORMANCE_DATA up front with an invalid-handle error, mirroring CPython’s special handling. (documentation.help)
    • Opens a child key when sub_key is non-empty, reads the unnamed (default) value via RegQueryValueExW, and correctly loops on ERROR_MORE_DATA by resizing the buffer and retrying.
    • Returns an empty string when RegQueryValueExW reports ERROR_FILE_NOT_FOUND, matching CPython’s documented behavior for a missing default value. (documentation.help)
    • Ensures the type is REG_SZ and then converts the UTF‑16 buffer to String, trimming at the first NUL.

Overall, this helper + QueryValue implementation looks good and behaviourally compatible with CPython.

Also applies to: 685-772

61-74: HKEYArg conversion is straightforward and useful

Allowing both PyHKEYObject and raw integer handles via TryFromObject is a nice compatibility affordance with CPython’s winreg, and the implementation is clear and side‑effect‑free.

No issues from a correctness or ergonomics standpoint.

174-190: PyHKEYObject lifecycle and context‑manager behavior look good

  • handle, __bool__, and __int__ simply expose the underlying handle and null-ness; they’re cheap and match CPython expectations for truthiness and integer conversion of HKEYs. (documentation.help)
  • __enter__/__exit__ delegate to Close, giving idiomatic context‑manager semantics.
  • The use of AtomicCell plus swap in Close/Detach/Drop correctly ensures:
    • Handles are closed exactly once.
    • Explicit Close and Detach prevent Drop from double‑closing by nulling out the stored handle before Drop runs.

Once the HKEY type/null sentinel issues from the previous comment are addressed, the lifecycle logic itself is sound.

Also applies to: 219-227, 230-237

268-307: Numeric protocol integration for PyHKEYObject is consistent

AsNumber is wired so that:

  • All arithmetic/bitwise operations fail with a uniform TypeError (“bad operand type”), which matches the non‑numeric nature of handles.
  • boolean delegates to __bool__ and int delegates to __int__, which is exactly what Python code using the numeric protocol would expect from bool(hkey) / int(hkey).

This is a clean and minimal numeric interface for a handle type.

309-341: ConnectRegistry, CreateKey, and CloseKey wrappers look correct

  • ConnectRegistry:

    • Handles both computer_name=None (local) and Some(String) (remote).
    • Uses to_wide_with_nul() for the remote machine name and passes NULL when None, per Win32 docs. (learn.microsoft.com)
    • Returns a fresh PyHKEYObject on success and maps non‑zero status to OSError.

  • CreateKey:

    • Calls RegCreateKeyW with the parent handle and UTF‑16 subkey name and wraps the returned handle.
    • Aligns with CPython’s winreg.CreateKey behavior. (docs.python.org)

  • CloseKey simply delegates to PyHKEYObject.Close, which centralizes close logic.

No functional issues here once the underlying HKEY representation is fixed as noted earlier.

Also applies to: 343-359, 401-405

406-415: Delete operations are wired correctly; ERROR_ACCESS_DENIED is usually expected for non‑empty keys

  • DeleteKey wraps RegDeleteKeyW with a UTF‑16 subkey name. Per Win32 docs, this function fails with ERROR_ACCESS_DENIED (5) if the subkey has subkeys; it cannot delete a key tree. (learn.microsoft.com)
  • DeleteKeyEx correctly calls RegDeleteKeyExW with (hKey, lpSubKey, samDesired, Reserved) now that the access/reserved order is fixed.
  • DeleteValue passes NULL for the value name when value=None, which the API treats as the unnamed/default value. (learn.microsoft.com)

If you are consistently seeing error code 5 from DeleteKey, double‑check that:

  • The target subkey has no subkeys.
  • You are pointing at the correct WOW64 view (for DeleteKeyEx) via access.
  • You’re not trying to delete HKEY_PERFORMANCE_DATA or other special views.

From the Rust side, these wrappers look correct; the remaining 5s are likely due to the documented Win32 constraints rather than an FFI bug.

Also applies to: 417-431, 433-461

463-490: EnumKey implementation matches Win32 and CPython expectations

  • Uses a 257‑character UTF‑16 buffer per the documented “255 chars + NUL, but 256 + no NUL is observed” behavior.
  • Calls RegEnumKeyExW and returns OSError on non‑zero status, matching CPython’s approach to index‑out‑of‑range and other errors. (documentation.help)
  • Converts only the returned character count to UTF‑16 string, with proper error mapping for invalid data.

No issues spotted here.

492-581: EnumValue logic and buffer‑growth handling look solid

  • Uses RegQueryInfoKeyW to determine maximum value name and data lengths, then allocates buffers with room for terminators.
  • Properly handles ERROR_MORE_DATA from RegEnumValueW by doubling the buffers and retrying, updating the “current size” inputs each iteration.
  • Uses the actual current_data_size when slicing the data buffer and then delegates to reg_to_py for type‑aware conversion.
  • Returns the expected (name, data, type) tuple.

Once the underlying HKEY type/“null handle” issues are corrected as noted earlier, this function is well‑structured and close to CPython’s algorithm.

585-592: FlushKey, LoadKey, and SaveKey are thin but correct wrappers

Each of these functions:

  • Calls the corresponding wide Win32 API (RegFlushKey, RegLoadKeyW, RegSaveKeyW) with the raw handle and UTF‑16 file/key names.
  • Treats any non‑zero return as an OSError with the raw code.

This is appropriate for RustPython; any additional privilege/SE_BACKUP/SE_RESTORE checks would normally be the caller’s responsibility in Python space.

Also applies to: 595-610, 848-859

612-647: OpenKey/OpenKeyEx look correct; good use of helper error mapping

  • OpenKeyArgs mirrors CPython’s signature (key, sub_key, reserved, access), with KEY_READ as the default access mask. (learn.microsoft.com)
  • OpenKey performs a single RegOpenKeyExW call with a UTF‑16 subkey name and wraps the returned HKEY in PyHKEYObject.
  • Errors are passed through os_error_from_windows_code, ensuring appropriate subclasses (e.g. FileNotFoundError, PermissionError) are raised.

One small consistency tweak: now that PyHKEYObject::new exists, you could use Ok(PyHKEYObject::new(res)) here for symmetry with other constructors.

649-683: QueryInfoKey implementation is straightforward and matches docs

  • Calls RegQueryInfoKeyW to obtain the subkey count, value count, and last write time.
  • Combines FILETIME.dwHighDateTime and dwLowDateTime into a single 64‑bit timestamp and returns (subkey_count, value_count, last_write_time).

This matches CPython’s winreg.QueryInfoKey contract. (documentation.help)

873-931: SetValue key handling and UTF‑16 encoding look correct

Beyond the small new_type_error issue:

  • Rejects HKEY_PERFORMANCE_DATA up front with ERROR_INVALID_HANDLE, which matches CPython’s special‑casing. (documentation.help)
  • Creates the subkey with RegCreateKeyExW (when sub_key is non‑empty) using KEY_SET_VALUE, then sets the unnamed value with RegSetValueExW + UTF‑16LE encoded string including terminator.
  • Closes the child key if it was created.

The use of to_wide_with_nul and (wide_value.len() * 2) byte count is correct for registry REG_SZ values.

933-962: reg_to_py covers the main registry types correctly

  • REG_DWORD / REG_QWORD: read up to 4/8 bytes with from_ne_bytes, returning 0 when data is too short, which mirrors CPython’s forgiving behavior.
  • REG_SZ / REG_EXPAND_SZ: convert via bytes_as_wide_slice, trim at the first NUL, and map UTF‑16 decoding errors to ValueError.
  • REG_MULTI_SZ: handle empty data as [], strip the trailing NUL if present, split on embedded NULs, and return a list of decoded strings.
  • Other / unknown types, including REG_BINARY, return a bytes object when data is non‑empty and None when empty.

Once the minor alignment concerns in bytes_as_wide_slice are addressed (see earlier comment), this conversion function looks comprehensive and faithful to CPython’s winreg behavior. (documentation.help)

Also applies to: 964-996

1008-1095: py2reg integer and string conversions are robust and CPython‑like

Highlights:

  • REG_DWORD / REG_QWORD:

    • Accept None as zero, matching CPython. (documentation.help)
    • Require a PyInt, rejecting non‑integers with TypeError.
    • Use Sign::Minus and to_u32/to_u64 to detect negative or too‑large values and raise OverflowError instead of panicking.

  • REG_SZ / REG_EXPAND_SZ:

    • Treat None as an empty string (a single UTF‑16 NUL), again matching CPython behavior.
    • Accept only str values and convert via UTF‑16LE, including the trailing NUL.

  • REG_MULTI_SZ:

    • Treat None as an empty list (double NUL terminator).
    • Enforce a list of strings and correctly concatenate UTF‑16LE strings with NUL separators and a final double‑NUL.

  • For REG_BINARY/others:

    • None maps to “no data” (so SetValueEx passes length 0 and null pointer).
    • Only accepts bytes for non‑None values, with a clear TypeError otherwise.

This is a solid, well‑guarded implementation.

1179-1210: ExpandEnvironmentStrings implementation looks correct and robust

  • Uses the wide ExpandEnvironmentStringsW API, first with lpDst = NULL/size = 0 to obtain the required size, then with an appropriately sized Vec<u16> buffer. (learn.microsoft.com)
  • Checks for 0 return as failure from both calls.
  • Trims at the first NUL and converts UTF‑16 to String, mapping decode errors to ValueError.

This is a clean, CPython‑like implementation with correct buffer sizing and error handling.

361-371: Out‑parameter and HKEY typing in CreateKeyEx are incorrect

CreateKeyEx has low‑level issues with RegCreateKeyExW's output parameter handling:

  • RegCreateKeyExW's last "phkResult" parameter expects PHKEY (*mut HKEY), but the code currently uses *mut std::ffi::c_void instead of the proper Registry::HKEY type. With windows_sys, HKEY is isize, so the output parameter should be a local Registry::HKEY, passed as &mut hkey to the API and then stored in PyHKEYObject.

  • This mirrors the bug fixed earlier in OpenKey: using a null *mut *mut c_void as the out‑pointer causes undefined behavior if the API writes through it.

Replace the *mut std::ffi::c_void declaration with let mut res: Registry::HKEY = 0; and use PyHKEYObject::new(res) for consistency with OpenKey.

861-871: SetValue error creation may have signature mismatch with new_type_error

The code passes a string literal to vm.new_type_error():

if typ != Registry::REG_SZ {
    return Err(vm.new_type_error("type must be winreg.REG_SZ"));
}

If new_type_error expects a String parameter (as suggested by other calls in this file using .to_string()), apply:

-    if typ != Registry::REG_SZ {
-        return Err(vm.new_type_error("type must be winreg.REG_SZ"));
-    }
+    if typ != Registry::REG_SZ {
+        return Err(vm.new_type_error("type must be winreg.REG_SZ".to_string()));
+    }

Verify the actual signature of vm.new_type_error() and confirm whether string literal is accepted or if .to_string() is required for consistency with the rest of the file.

95-113: Handle representation, predefined keys, and __str__ need tightening

There are a few intertwined issues around PyHKEYObject and HKEY constants:

  1. HKEY type vs raw pointers

    In windows_sys, HKEY is defined as isize, not *mut c_void. The code appears to treat HKEYs as raw pointers in multiple locations, which will cause type mismatches or rely on unintended coercions. Refactor to treat HKEYs as the integer handle type that windows_sys exposes.

  2. Predefined HKEY constants being closed

    Predefined constants (HKEY_CLASSES_ROOT, HKEY_CURRENT_USER, etc.) must not be closed via RegCloseKey as Windows documentation specifies the handle must have been opened by RegCreateKeyEx, RegOpenKeyEx, RegConnectRegistry, etc. Add a flag (e.g. is_predefined: bool) or separate constructor for predefined keys and skip RegCloseKey in Drop/Close.

  3. __str__ formatting as a pointer

    __str__ currently formats with {:p} which does not type-check with HKEY = isize. Use hex integer representation instead: format!("<PyHKEY:0x{:x}>", self.hkey.load() as usize).