linux-kernel - Re: [PATCH net-next v3 2/8] rust: time: Introduce Delta type

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CANiq72msOTdVLjX+7+Xx4Si2Sh=s1M=wrg_T+QkpFyBHSC9gwA@mail.gmail.com>
Date: Sun, 20 Oct 2024 15:05:36 +0200
From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: FUJITA Tomonori <fujita.tomonori@...il.com>, netdev@...r.kernel.org, 
	rust-for-linux@...r.kernel.org, hkallweit1@...il.com, tmgross@...ch.edu, 
	ojeda@...nel.org, alex.gaynor@...il.com, gary@...yguo.net, 
	bjorn3_gh@...tonmail.com, benno.lossin@...ton.me, a.hindborg@...sung.com, 
	aliceryhl@...gle.com, anna-maria@...utronix.de, frederic@...nel.org, 
	tglx@...utronix.de, arnd@...db.de, jstultz@...gle.com, sboyd@...nel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v3 2/8] rust: time: Introduce Delta type

On Sat, Oct 19, 2024 at 8:41 PM Andrew Lunn <andrew@...n.ch> wrote:
>
> So far, we only have one use case for this type, holding a duration to
> be passed to fsleep(). Rounding down what you pass to fsleep() is
> generally not what the user wants to do, and we should try to design
> the code to avoid this. The current helpers actually encourage such
> bugs, because they round down. Because of this they are currently
> unused. But they are a trap waiting for somebody to fall into. What
> the current users of this type really want is lossy helpers which
> round up. And by reviewing the one user against the API, it is clear
> the current API is wrong.

If you are talking about Rust's `fsleep()`, then "users" are not the
ones calling the rounding up operation (the implementation of
`fsleep()` is -- just like in the C side).

If you are talking about C's `fsleep()`, then users are not supposed
to use `bindings::`.

> So i say, throw away the round down helpers until somebody really
> needs them. That avoids a class of bugs, passing a too low value to
> sleep. Add the one helper which is actually needed right now.

Eventually this type will likely get other methods for one reason or
another, including the non-rounding ones. Thus we should be careful
about the names we pick, which is why I was saying a method like
`as_micros()` should not be rounding up. That would be confusing, and
thus potentially end creating bugs, even if it is the only method you
add today.

Again, if you want to throw away all the unused methods and only have
the rounding up one, then that is reasonable, but please let's not add
misleading methods that could add more bugs than the ones you are
trying to avoid. Please use `as_micros_ceil()` or similar.

> There is potentially a better option. Make the actual sleep operation
> part of the type. All the rounding up then becomes part of the core,
> and the developer gets core code which just works correctly, with an
> API which is hard to make do the wrong thing.

That is orthogonal: whether the sleeping function is in `Delta` or
not, users would not be the ones calling the rounding up operation
(please see above).

Anyway, by moving more things into a type, you are increasing its
complexity. And, depending how modules are organized, you could be
also giving visibility into the internals of that type, thus
increasing the possibility of "implementation-side bugs" compared to
the other way around (it does not really matter here, though).

If you want it as a method for user ergonomics though, that is a
different topic.

Cheers,
Miguel