Re: [PATCH 2/2] rust: sync: add `CondVar::wait_timeout`

[Alice Ryhl]

On Fri, Dec 8, 2023 at 10:27 AM Benno Lossin <benno.lossin@xxxxxxxxx> wrote:
>
> On 12/8/23 08:37, Alice Ryhl wrote:
> > On Wed, Dec 6, 2023 at 6:05 PM Tiago Lam <tiagolam@xxxxxxxxx> wrote:
> >> On 06/12/2023 10:09, Alice Ryhl wrote:
> >>> +/// The return type of `wait_timeout`.
> >>> +pub enum CondVarTimeoutResult {
> >>> +    /// The timeout was reached.
> >>> +    Timeout,
> >>> +    /// Somebody woke us up.
> >>> +    Woken {
> >>> +        /// Remaining sleep duration.
> >>> +        jiffies: u64,
> >>> +    },
> >>> +    /// A signal occurred.
> >>> +    Signal {
> >>> +        /// Remaining sleep duration.
> >>> +        jiffies: u64,
> >>> +    },
> >>> +}
> >>
> >> Is `Signal` and `Woken` only going to hold a single value? Would it be
> >> best represented as a tuple struct instead, like so?
> >>
> >>      pub enum CondVarTimeoutResult {
> >>          /// The timeout was reached.
> >>          Timeout,
> >>          /// Somebody woke us up.
> >>          Woken (u64),
> >>          /// A signal occurred.
> >>          Signal (u64),
> >>      }
> >
> > I could do that, but I like the explicitly named version as it makes
> > it clear that the unit is jiffies.
>
> Why not use `type Jiffies = u64;` until we have proper bindings for
> them? That way we can have both.

I do not mind adding and using a type alias, but I still think the
named fields are better.

Alice