Re: [PATCH v4 2/3] rust: time: make Delta division and remainder fail consistently

[chenhan <chenhan0017.work@gmail.com>]

Hi Andreas,

Thanks for asking. I should have distinguished the non-zero-divisor
requirement from the MIN/-1 check in those comments.

> Why change the safety comment here? If the function is always safe to
> call, the exclusion is not relevant. Was the old comment wrong?

The old "regardless of the input values" statement is too broad. In the
generic 32-bit implementation, div64_s64() goes through div64_u64() and
div_u64_rem() to do_div(). The latter contains C division and remainder
operations which require a non-zero divisor. The kernel's
-fno-strict-overflow does not define division by zero. An architecture's
divide-by-zero handling therefore does not establish that the C call is
safe for every input across implementations.

The zero-divisor check above the call supplies that precondition. I have
updated the local comments for the next revision to cite that check
explicitly, without grouping it with MIN/-1. For the division call:

    // SAFETY: `rhs.value` is non-zero due to the check above.

The MIN/-1 check preserves Rust's panic semantics; the generic
32-bit helpers use unsigned magnitude division and wrapping sign
arithmetic, so I should not present its rejection as a necessary safety
precondition of those implementations.

> Again, was the old safety comment wrong?

The old pointer explanation was correct: the local rem supplies valid
storage for the output. I expanded it to make the validity, alignment
and writability of &mut rem explicit. The signed remainder helper also
reaches the division path above, so the comment should explain both
the non-zero divisor, established by the check, and the valid output
pointer. The updated comments keep these separate from the MIN/-1
behavior check; both runtime checks are unchanged.

I have made the corresponding comment changes in the math helpers in
patch 3 as well, so the explanation remains consistent after extraction.

> Once kunit supports the `#[should_panic]` attribute, we can assert the
> behavior on all platforms.

Agreed; that would let us keep these panic cases as in-tree regression
tests when the support is available.

Best regards,
chenhan