Re: [PATCH] rust: time: Seal the ClockSource trait

[FUJITA Tomonori]

On Wed, 18 Jun 2025 21:13:07 +0200
Andreas Hindborg <a.hindborg@xxxxxxxxxx> wrote:

> "Boqun Feng" <boqun.feng@xxxxxxxxx> writes:
> 
>> On Tue, Jun 17, 2025 at 05:10:42PM -0700, Boqun Feng wrote:
>>> On Wed, Jun 18, 2025 at 08:20:53AM +0900, FUJITA Tomonori wrote:
>>> > Prevent downstream crates or drivers from implementing `ClockSource`
>>> > for arbitrary types, which could otherwise leads to unsupported
>>> > behavior.
>>> >
>>>
>>> Hmm.. I don't think other impl of `ClockSource` is a problem, IIUC, as
>>> long as the ktime_get() can return a value in [0, i64::MAX). Also this
>>> means ClockSource should be an `unsafe` trait, because the correct
>>> implementaion relies on ktime_get() returns the correct value. This is
>>> needed even if you sealed ClockSource trait.
>>>
>>> Could you drop this and fix that the ClockSource trait instead? Thanks!
>>>
>>
>> For example:
>>
>>     /// Trait for clock sources.
>>     ///
>>     /// ...
>>     /// # Safety
>>     ///
>>     /// Implementers must ensure `ktime_get()` return a value in [0,
>>     //  KTIME_MAX (i.e. i64::MAX)).
>>     pub unsafe trait ClockSource {
>>         ...
>>     }
> 
> Nice catch, it definitely needs to be unsafe. We should also require
> correlation between ID and the value fetched by `ktime_get`.

What's ID?