Re: [PATCH v2] rust: time: make Delta division and remainder fail consistently

[Miguel Ojeda]

On Thu, Oct 1, 2026 at 6:29 AM chenhan <chenhan0017.work@xxxxxxxxx> wrote:
>
> commit. Correct the rem_nanos() parameter name to divisor as well.

This seems like it should be a first patch.

> Link: https://github.com/Rust-for-Linux/linux/issues/1254#issuecomment-5869573842
> Link: https://github.com/Rust-for-Linux/linux/issues/1254#issuecomment-5869925737

The discussion is small enough, so I am not sure why we need the two
Link:s. They can be useful if you want to reference something in
particular, using e.g. the square brackets notation [1] to point to
them.

> On the MIN / -1 safety question: div_s64_rem() computes a quotient even
> though the caller discards it. V1 already checked this input before the C
> call; v2 keeps the guard and makes the representable-quotient condition
> explicit in the SAFETY comment.

Yes, that is fine -- what I meant is that even if we had the check,
the `// SAFETY` comment should still justify why it is not UB (please
see below).

> -            // SAFETY: This function is always safe to call regardless of the input values
> +            // SAFETY: The divisor is non-zero and the quotient is representable.

"representable" is a fine way to succinctly put it, but I wonder if it
would be better to just spell the condition.

In any case, safety comment need to explain *why* those conditions are
true, not just re-state them (otherwise, safety comments would be a
repetition of the `# Safety` documentation of the function called, and
thus not useful). In other words, the point is that they explain why
something is supposed to hold.

> -    /// Return `self % dividend` where `dividend` is in nanoseconds.
> +    /// Return `self % divisor`, where `divisor` is a number of nanoseconds.

"a number of" seems to be added spuriously -- please try not to do
unrelated changes, or you do them, please add as new commits (e.g.
this one wouldn't be a fix to backport).

> +    /// The divisor is an [`i32`] because `div_s64_rem()`, used on 32-bit
> +    /// platforms, takes a signed 32-bit divisor. The dividend remains an [`i64`].

Do we need this in the public documentation?

> +    /// Panics if `divisor` is zero, or if `self` represents [`i64::MIN`]
> +    /// nanoseconds and `divisor` is `-1`, matching Rust's signed remainder
> +    /// operator even though the remainder would be zero.

Not sure if we need the "even though ..." bit.

Thanks!

Cheers,
Miguel