fix(windows): ensure memset_explicit symbol is properly exported for MinGW #119

Merged
sarman merged 1 commits from fix/cargo-config-sodium into beta 2026-06-19 14:54:50 +00:00
Owner
  • Add MINGW32 detection in memset_s_shim.c for proper EXPORT macro
  • Add WIN32 and WIN32 defines when compiling the shim in build.rs
  • The memset_explicit symbol is required by libsodium-sys-stable but not
    available in MinGW runtime
  • The shim is compiled as a static library and linked before libsodium
- Add __MINGW32__ detection in memset_s_shim.c for proper EXPORT macro - Add WIN32 and __WIN32__ defines when compiling the shim in build.rs - The memset_explicit symbol is required by libsodium-sys-stable but not available in MinGW runtime - The shim is compiled as a static library and linked before libsodium
sarman added 1 commit 2026-06-19 14:48:58 +00:00
fix(windows): ensure memset_explicit symbol is properly exported for MinGW
Some checks failed
Test / frontend-tests (pull_request) Successful in 1m45s
Test / frontend-typecheck (pull_request) Successful in 1m57s
PR Review Automation / review (pull_request) Successful in 4m23s
Test / rust-fmt-check (pull_request) Has been cancelled
Test / rust-clippy (pull_request) Has been cancelled
Test / rust-tests (pull_request) Has been cancelled
5c4fb05802
- Add __MINGW32__ detection in memset_s_shim.c for proper EXPORT macro
- Add WIN32 and __WIN32__ defines when compiling the shim in build.rs
- The memset_explicit symbol is required by libsodium-sys-stable but not
  available in MinGW runtime
- The shim is compiled as a static library and linked before libsodium
sarman reviewed 2026-06-19 14:53:21 +00:00
sarman left a comment
Author
Owner

Automated PR Review (qwen3.5-122b-think via liteLLM):\n\n

Summary

The PR addresses a Windows MinGW build issue where memset_explicit from libsodium-sys-stable isn't resolved. The changes add a C shim with proper export macros and compile it conditionally for Windows MinGW. However, there's a critical link order issue that could cause the fix to fail.

Findings

  • [WARNING] src-tauri/build.rs:25 - Link order not guaranteed
    Evidence: println!("cargo:rustc-link-lib=static=memset_shim");
    Fix: The comment states "must be before libsodium in link order" but there's no mechanism to enforce this. Cargo doesn't guarantee link order with just rustc-link-lib. Consider using cargo:rustc-link-search or restructuring the build to ensure memset_shim links before libsodium.

  • [WARNING] src-tauri/memset_s_shim.c:15 - Inconsistent macro detection
    Evidence: #if defined(_WIN32) || defined(__WIN32__) || defined(__MINGW32__)
    Fix: The build.rs only defines WIN32 and __WIN32__, but the C file checks for __MINGW32__ which isn't defined in build.rs. Either add .define("__MINGW32__", None) in build.rs or remove __MINGW32__ from the C file's macro check.

  • [SUGGESTION] src-tauri/memset_s_shim.c:1 - Naming inconsistency
    Evidence: File is memset_s_shim.c but provides memset_explicit
    Fix: Consider renaming to memset_explicit_shim.c for clarity, though this is not functionally problematic.

Verdict: APPROVE WITH COMMENTS

Automated PR Review (qwen3.5-122b-think via liteLLM):\n\n **Summary** The PR addresses a Windows MinGW build issue where `memset_explicit` from libsodium-sys-stable isn't resolved. The changes add a C shim with proper export macros and compile it conditionally for Windows MinGW. However, there's a critical link order issue that could cause the fix to fail. **Findings** - [WARNING] src-tauri/build.rs:25 - Link order not guaranteed Evidence: `println!("cargo:rustc-link-lib=static=memset_shim");` Fix: The comment states "must be before libsodium in link order" but there's no mechanism to enforce this. Cargo doesn't guarantee link order with just `rustc-link-lib`. Consider using `cargo:rustc-link-search` or restructuring the build to ensure memset_shim links before libsodium. - [WARNING] src-tauri/memset_s_shim.c:15 - Inconsistent macro detection Evidence: `#if defined(_WIN32) || defined(__WIN32__) || defined(__MINGW32__)` Fix: The build.rs only defines `WIN32` and `__WIN32__`, but the C file checks for `__MINGW32__` which isn't defined in build.rs. Either add `.define("__MINGW32__", None)` in build.rs or remove `__MINGW32__` from the C file's macro check. - [SUGGESTION] src-tauri/memset_s_shim.c:1 - Naming inconsistency Evidence: File is `memset_s_shim.c` but provides `memset_explicit` Fix: Consider renaming to `memset_explicit_shim.c` for clarity, though this is not functionally problematic. **Verdict**: APPROVE WITH COMMENTS
sarman merged commit 036af81147 into beta 2026-06-19 14:54:50 +00:00
sarman deleted branch fix/cargo-config-sodium 2026-06-19 14:54:50 +00:00
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: sarman/tftsr-devops_investigation#119
No description provided.