Thread (63 messages) 63 messages, 5 authors, 2025-01-24

Re: [PATCH v8 2/7] rust: time: Introduce Delta type

From: FUJITA Tomonori <fujita.tomonori@gmail.com>
Date: 2025-01-22 07:37:38
Also in: lkml, rust-for-linux

On Sat, 18 Jan 2025 13:19:21 +0100
Miguel Ojeda [off-list ref] wrote:
Since you started getting Reviewed-bys, I don't want to delay this
more (pun unintended :), but a couple quick notes...

I can create "good first issues" for these in our tracker if you
prefer, since these should be easy and can be done later (even if, in
general, I think we should require examples and good docs for new
abstractions).
Yes, please create such. I'll add more docs but I'm sure that there
will be room for improvement.
On Thu, Jan 16, 2025 at 5:42 AM FUJITA Tomonori
[off-list ref] wrote:
quoted
i64 is used instead of bindings::ktime_t because when the ktime_t
type is used as timestamp, it represents values from 0 to
KTIME_MAX, which different from Delta.
Typo: "is different ..."?
Oops, will fix.
quoted
Delta::from_[millis|secs] APIs take i64. When a span of time
overflows, i64::MAX is used.
This behavior should be part of the docs of the methods below.
You want to add the above explanation to all the
Delta::from_[millis|micro|secs], right?
quoted
+/// A span of time.
+#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Debug)]
+pub struct Delta {
+    nanos: i64,
+}
Some more docs here in `Delta` would be good, e.g. some questions
readers may have could be: What range of values can it hold? Can they
be negative?
Okay, I'll add.
Also some module-level docs would be nice relating all the types (you
mention some of that in the commit message for `Instant`, but it would
be nice to put it as docs, rather than in the commit message).
Is there any existing source code I can refer to? I'm not sure
how "module-level docs" should be written.
quoted
+    /// Create a new `Delta` from a number of microseconds.
+    #[inline]
+    pub const fn from_micros(micros: i64) -> Self {
+        Self {
+            nanos: micros.saturating_mul(NSEC_PER_USEC),
+        }
+    }
For each of these, I would mention that they saturate and I would
mention the range of input values that would be kept as-is without
loss.

And it would be nice to add some examples, which you can take the
chance to show how it saturates, and they would double as tests, too.
I'll try to improve.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help