linux-kernel - Re: [PATCH v1] rust: time: Add examples with doctest for Delta

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87bjq0tnxr.fsf@kernel.org>
Date: Fri, 04 Jul 2025 09:28:16 +0200
From: Andreas Hindborg <a.hindborg@...nel.org>
To: "FUJITA Tomonori" <fujita.tomonori@...il.com>
Cc: <miguel.ojeda.sandonis@...il.com>,  <alex.gaynor@...il.com>,
  <ojeda@...nel.org>,  <aliceryhl@...gle.com>,  <anna-maria@...utronix.de>,
  <bjorn3_gh@...tonmail.com>,  <boqun.feng@...il.com>,  <dakr@...nel.org>,
  <frederic@...nel.org>,  <gary@...yguo.net>,  <jstultz@...gle.com>,
  <linux-kernel@...r.kernel.org>,  <lossin@...nel.org>,
  <lyude@...hat.com>,  <rust-for-linux@...r.kernel.org>,
  <sboyd@...nel.org>,  <tglx@...utronix.de>,  <tmgross@...ch.edu>
Subject: Re: [PATCH v1] rust: time: Add examples with doctest for Delta

"FUJITA Tomonori" <fujita.tomonori@...il.com> writes:

> On Wed, 2 Jul 2025 14:22:48 +0200
> Miguel Ojeda <miguel.ojeda.sandonis@...il.com> wrote:
>
>> On Wed, Jul 2, 2025 at 10:36 AM Andreas Hindborg <a.hindborg@...nel.org> wrote:
>>>
>>> I think this kind of test would be better suited for the new `#[test]`
>>> macro. Would you agree?
>>
>> I don't mind seeing the edge cases, since the behavior is mentioned in
>> the docs, just like sometimes we show e.g. the `Err`/`None` cases in
>> other functions etc., and it may help to further highlight that this
>> can actually return possibly unexpected values.
>>
>> It is also what the standard library does, at least in some similar cases, e.g.
>>
>>     https://doc.rust-lang.org/std/primitive.i64.html#method.saturating_add
>>
>> Maybe instead we can help making it a bit more readable, e.g. avoiding
>> the intermediate variable to have a single line plus using a `# use
>> Delta` to further reduce the line length?
>>
>> Also adding a newline between the "normal" case and the saturation
>> cases would probably help too.
>
> I've updated from_micros() based on the above suggestion - looks
> better to me. What do you think?
>
> /// # Examples
> ///
> /// ```
> /// use kernel::time::Delta;
> ///
> /// assert_eq!(Delta::from_micros(5).as_nanos(), 5_000);
> /// assert_eq!(Delta::from_micros(9_223_372_036_854_775).as_nanos(), 9_223_372_036_854_775_000);
> /// assert_eq!(Delta::from_micros(-9_223_372_036_854_775).as_nanos(), -9_223_372_036_854_775_000);
> ///
> /// assert_eq!(Delta::from_micros(9_223_372_036_854_776).as_nanos(), i64::MAX);
> /// assert_eq!(Delta::from_micros(-9_223_372_036_854_776).as_nanos(), i64::MIN);
> /// ```
> #[inline]
> pub const fn from_micros(micros: i64) -> Self {
>     Self {
>         nanos: micros.saturating_mul(NSEC_PER_USEC),
>     }
> }

>From my point of view, I would like to see

  assert_eq!(Delta::from_micros(9_223_372_036_854_775).as_nanos(), 9_223_372_036_854_775_000);
  assert_eq!(Delta::from_micros(-9_223_372_036_854_775).as_nanos(), -9_223_372_036_854_775_000);

  assert_eq!(Delta::from_micros(9_223_372_036_854_776).as_nanos(), i64::MAX);
  assert_eq!(Delta::from_micros(-9_223_372_036_854_776).as_nanos(), i64::MIN);

moved to a `#[test]` block. They do not provide value for me when
reading the docs. I don't know what the very large constants are and
what I am supposed to learn from those asserts. Maybe if the constants
had a name, or were expressed relative to another constant?

I think this one:

  /// assert_eq!(Delta::from_micros(5).as_nanos(), 5_000);

is fine in the documentation block.


Best regards,
Andreas Hindborg