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

[Andreas Hindborg <a.hindborg@kernel.org>]

"Boqun Feng" <boqun.feng@gmail.com> 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`.

But I still think it is fine to seal the trait, why not?


Best regards,
Andreas Hindborg