Document number casting safety across platform boundaries by Copilot · Pull Request #78 · Nugine/rlimit

Copilot · 2025-12-14T19:08:12Z

Added safety documentation for all type conversions between Rust and libc types, particularly for cross-platform scenarios where type sizes vary.

Changes

unix.rs - rlim_t ↔ u64 conversions

Values clamped to INFINITY before downcasting prevents truncation on 32-bit platforms
Documented that upcasts from rlim_t are lossless (u64→u64) or widening (u32→u64)

windows.rs - c_int ↔ u32 conversions

Validation ensures new_max ≤ c_int::MAX before casting
Return value checked non-negative before unsigned cast

tools.rs - c_int → u64 conversion

Non-negative sysctl result always widens safely

Added inline SAFETY comments explaining the safety rationale for all casts without introducing unnecessary clippy allow attributes.

// Before: unclear if cast is safe
let rlim = C::rlimit {
    rlim_cur: soft.min(INFINITY) as _,
    rlim_max: hard.min(INFINITY) as _,
};

// After: explicit safety guarantee
// SAFETY: Values clamped to INFINITY (platform max) before casting.
// On 64-bit: u64→u64 (no-op). On 32-bit: u64→u32 after clamping.
let rlim = C::rlimit {
    rlim_cur: soft.min(INFINITY) as _,
    rlim_max: hard.min(INFINITY) as _,
};

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Original prompt

This section details on the original issue you should resolve

<issue_title>Review number casting</issue_title>
<issue_description></issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes Review number casting #77

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: Nugine <30099658+Nugine@users.noreply.github.com>

Nugine · 2025-12-15T15:01:12Z

@copilot Try to avoid clippy warnings without explicit allowance.

Co-authored-by: Nugine <30099658+Nugine@users.noreply.github.com>

Copilot · 2025-12-15T15:04:38Z

@copilot Try to avoid clippy warnings without explicit allowance.

Done in dda9a35. Removed the new #[allow(...)] attributes I added - they weren't needed as clippy doesn't warn about these casts. Kept the documentation comments explaining the safety rationale.

Copilot

Pull request overview

This PR adds comprehensive safety documentation for numeric type conversions between Rust and C/libc types across different platforms. The changes explain why casts between types like u64 ↔ rlim_t (Unix) and u32 ↔ c_int (Windows) are safe, with particular attention to platform-specific type size variations.

Key changes:

Added SAFETY comments explaining the rationale for all numeric casts involving platform-dependent types
Documented that clamping to INFINITY prevents truncation issues on 32-bit platforms
Clarified that validation checks ensure casts behave correctly

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
src/unix.rs	Documents `rlim_t ↔ u64` conversion safety with INFINITY clamping for cross-platform compatibility
src/windows.rs	Documents `c_int ↔ u32` conversion safety with validation and sign checks
src/tools.rs	Documents `c_int → u64` widening conversion safety for sysctl return values

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-15T16:37:02Z

src/windows.rs

+    // SAFETY: We've validated that new_max fits in c_int, so the cast is safe
+    // (though clippy warns about possible wrapping). The cast back to u32 is
+    // safe because we've checked that ret >= 0.


The SAFETY comment here is misleading. The cast on line 38 (new_max as c_int) is not made safe by the validation check - the check simply ensures the cast won't wrap around to a negative value. The actual cast can still wrap (which is why clippy warns about it), but we've verified that the wrapped value would be correct for this specific case.

Similarly, the cast on line 42 (ret as u32) is not made "safe because we've checked that ret >= 0" - the non-negativity check prevents sign loss from being a problem, but the cast itself still involves sign loss.

The comment should clarify that the validation ensures the cast behaves correctly (doesn't produce a negative number), rather than claiming the cast itself becomes "safe".

Copilot · 2025-12-15T16:37:02Z

src/windows.rs

+    // SAFETY: c_int::MAX (i32::MAX = 2147483647) fits in u32, so this cast is safe.
    if new_max > c_int::MAX as u32 {


The SAFETY comment on line 25 is misplaced and potentially confusing. It's attached to a validation check that prevents overflow, not to an actual unsafe cast. The cast on line 26 (c_int::MAX as u32) is indeed safe since i32::MAX fits in u32, but this comment should be placed directly above line 26 where the cast occurs, not above the conditional statement. Additionally, the comment should clarify that it's explaining why the comparison operation's cast is safe, not why the validation itself is safe.

Suggested change

// SAFETY: c_int::MAX (i32::MAX = 2147483647) fits in u32, so this cast is safe.

if new_max > c_int::MAX as u32 {

if new_max > {

// SAFETY: c_int::MAX (i32::MAX = 2147483647) fits in u32, so this cast in the comparison is safe.

c_int::MAX as u32

} {

Copilot · 2025-12-15T16:37:03Z

src/unix.rs

+/// This constant is the minimum of the platform's `RLIM_INFINITY` and `u64::MAX`.
+/// On most platforms, `RLIM_INFINITY` is already `u64`, making this cast lossless.


The documentation states "This constant is the minimum of the platform's RLIM_INFINITY and u64::MAX", but this is technically imprecise. The constant is actually the result of casting RLIM_INFINITY to u64, which is not the same as taking the minimum. On platforms where rlim_t is u32, this cast widens the value (e.g., u32::MAX becomes u64 equivalent), not takes a minimum. The comment should say something like "This constant is RLIM_INFINITY cast to u64" or describe the actual behavior more accurately.

Suggested change

/// This constant is the minimum of the platform's `RLIM_INFINITY` and `u64::MAX`.

/// On most platforms, `RLIM_INFINITY` is already `u64`, making this cast lossless.

/// This constant is the platform's `RLIM_INFINITY` cast to `u64`.

/// On most platforms, `RLIM_INFINITY` is already `u64`, making this cast lossless.

/// On platforms where `rlim_t` is smaller (e.g., `u32`), this cast widens the value.

Copilot AI self-assigned this Dec 14, 2025

Initial plan

7aa5d98

Copilot AI assigned Nugine Dec 14, 2025

Copilot started work on behalf of Nugine December 14, 2025 19:08 View session

Copilot AI and others added 2 commits December 14, 2025 19:15

Add comprehensive documentation for number casting safety

4ce0853

Co-authored-by: Nugine <30099658+Nugine@users.noreply.github.com>

Simplify cast expressions per code review

4aa108c

Co-authored-by: Nugine <30099658+Nugine@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Review number casting implementation~~ Document number casting safety across platform boundaries Dec 14, 2025

Copilot AI requested a review from Nugine December 14, 2025 19:23

Copilot finished work on behalf of Nugine December 14, 2025 19:23

Copilot started work on behalf of Nugine December 15, 2025 15:01 View session

Remove unnecessary clippy allow attributes

dda9a35

Co-authored-by: Nugine <30099658+Nugine@users.noreply.github.com>

Copilot finished work on behalf of Nugine December 15, 2025 15:06

fix

75fb305

Nugine marked this pull request as ready for review December 15, 2025 15:21

Nugine requested review from Copilot and removed request for Copilot December 15, 2025 15:24

Nugine merged commit e7b4e0d into main Dec 15, 2025
35 checks passed

Nugine deleted the copilot/review-number-casting branch December 15, 2025 15:31

Copilot started reviewing on behalf of Nugine December 15, 2025 15:57 View session

Copilot started reviewing on behalf of Nugine December 15, 2025 16:05 View session

Copilot AI reviewed Dec 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Document number casting safety across platform boundaries#78

Document number casting safety across platform boundaries#78
Nugine merged 5 commits intomainfrom
copilot/review-number-casting

Copilot AI commented Dec 14, 2025 •

edited

Loading

Uh oh!

Nugine commented Dec 15, 2025

Uh oh!

Copilot AI commented Dec 15, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		// SAFETY: c_int::MAX (i32::MAX = 2147483647) fits in u32, so this cast is safe.
		if new_max > c_int::MAX as u32 {

		/// This constant is the minimum of the platform's `RLIM_INFINITY` and `u64::MAX`.
		/// On most platforms, `RLIM_INFINITY` is already `u64`, making this cast lossless.

Uh oh!

Conversation

Copilot AI commented Dec 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Comments on the Issue (you are @copilot in this section)

Uh oh!

Nugine commented Dec 15, 2025

Uh oh!

Copilot AI commented Dec 15, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Dec 14, 2025 •

edited

Loading