Re: [PATCH v4 3/3] rust: workqueue: add creation of workqueues

[Danilo Krummrich]

On Sat Mar 14, 2026 at 11:30 AM CET, Alice Ryhl wrote:
> On Thu, Mar 12, 2026 at 06:59:31PM +0100, Danilo Krummrich wrote:
>> On Thu Mar 12, 2026 at 10:23 AM CET, Alice Ryhl wrote:
>> > +impl Queue {
>> > +    /// Build a workqueue whose work may execute on any cpu.
>> > +    ///
>> > +    /// # Examples
>> > +    ///
>> > +    /// ```
>> > +    /// use kernel::workqueue::Queue;
>> > +    ///
>> > +    /// let wq = Queue::new_unbound().build(c"my-wq")?;
>> > +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from unbound wq"))?;
>> > +    /// # Ok::<(), Error>(())
>> > +    /// ```
>> > +    #[inline]
>> > +    #[doc(alias = "WQ_UNBOUND")]
>> > +    pub fn new_unbound() -> Builder<TypeUnbound> {
>> 
>> Wouldn't it be more straight forward if those where constructors of Builder?
>> 
>> 	let wq = wq::Builder::new_unbound().build(c"my-wq")?;
>
> Crates out there do both, but I actually like this pattern quite a lot.
> After all, given 'wq::Builder::new_unbound()' and 'Queue::new_unbound()'
> I think the latter is the one that makes it more clear that you are
> building a 'Queue'.
>
> Also, it avoids an extra import.

If it is about avoiding an extra import, what about

	Queue::builder_unbound()

this makes the name less misleading and avoids the import.

>> Queue::new_unbound() suggests that it returns a new Queue object, but instead it
>> returns a new Builder object.
>
> I'm only really concerned about confusion if it falls into one of these
> categories:
>
> * You write the wrong code, it compiles, and silently does the wrong
>   thing.
> * You read the code, and misunderstand what it does.
>
> I don't think this falls into either of those cases. If you forget the
> .build() call, all you get is a compiler error, and it's not hard to
> figure out what to do when you see the error.
>
> Alice