Re: [PATCH v8 4/7] rust: time: Add wrapper for fsleep function

From: Miguel Ojeda
Date: Wed Jan 22 2025 - 05:22:20 EST


On Wed, Jan 22, 2025 at 7:57 AM FUJITA Tomonori
<fujita.tomonori@xxxxxxxxx> wrote:
>
> Agreed that "the C side" is better and updated the comment. I copied
> that expression from the existing code; there are many "kernel's" in
> rust/kernel/. "good first issues" for them?

Yeah, will do.

> You prefer "[`fsleep()`]" rather than "[`fsleep`]"? I can't find any
> precedent for the C side functions.

There is no preference yet. It would be nice to be consistent, though.
The option of removing the `()` in all cases may be easier to check
for than the other, though the `()` give a bit of (possibly redundant)
information to the reader.

> Yeah, simpler is better. After applying the above changes, it ended up
> as follows.

Looks good, thanks!

Not sure if we should say "Equivalent" given it is not exactly the
same, but I am not a native speaker: I think it does not necessarily
need to be exactly the same to be "equivalent", but perhaps "Similar
to" or "Counterpart of" or something like that is better.

> Ah, it might work. The following doesn't work. Seems that we need to
> add another const like MAX_DELTA_NANOS or something. No strong
> preference but I feel the current is simpler.
>
> let delta = match delta.as_nanos() {
> 0..=MAX_DELTA.as_nanos() as i32 => delta,
> _ => MAX_DELTA,
> };

Yeah, don't worry about it too much :)

[ The language may get `const { ... }` to work there (it does in
nightly) though it wouldn't look good either. I think the `as i32`
would not be needed. ]

By the way, speaking of something related, do we want to make some of
the methods `fn`s be `const`?

Cheers,
Miguel