Re: [bug report] mux: Add helper functions for getting optional and selected mux-state

Dan Carpenter <[email protected]>
Newsgroups org.kernel.vger.kernel-janitors,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On Sun, Apr 19, 2026 at 10:16:30AM +0000, Josua Mayer wrote:
> Hi Dan,
> 
> Am 10.04.26 um 12:12 schrieb Dan Carpenter:
> > Hello Josua Mayer,
> >
> > Commit 993bcaf32c49 ("mux: Add helper functions for getting optional
> > and selected mux-state") from Feb 26, 2026 (linux-next), leads to the
> > following Smatch static checker warning:
> >
> > 	drivers/mux/core.c:640 mux_control_get()
> > 	warn: 'mux' is an error pointer or valid
> >
> > drivers/mux/core.c
> >     630  * mux_control_get() - Get the mux-control for a device.
> >     631  * @dev: The device that needs a mux-control.
> >     632  * @mux_name: The name identifying the mux-control.
> >     633  *
> >     634  * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
> >     635  */
> >     636 struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
> >     637 {
> >     638         struct mux_control *mux = mux_get(dev, mux_name, NULL, false);
> >
> > mux_get() can only return NULL if optional is true.
> 
> Yes, that is the intended contract. This means function can be simplified:
> 
> /**
>  * mux_control_get() - Get the mux-control for a device.
>  * @dev: The device that needs a mux-control.
>  * @mux_name: The name identifying the mux-control.
>  *
>  * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
>  */
> struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
> {
>     return mux_get(dev, mux_name, NULL, false);
> }
> EXPORT_SYMBOL_GPL(mux_control_get);
> 
> Is it okay to trust such transitive contracts and not check for NULL
> in an exported generic helper function?
> 

Yes.  If you don't pass "optional" then you don't need to check for
NULL.  If it's buggy, just fix it.  Predicting and working around future
bugs is never going to work.

> >
> >     639 
> > --> 640         if (!mux)
> >
> > this should be if (IS_ERR(mux)) {
> >
> >     641                 return ERR_PTR(-ENOENT);
> No, ENOENT is only the fix for unexpected NULL return,
> which must not be propagated to the caller.
> 
> Other errors should be returned to the caller unchanged.
> 

I wrote a blog explaining how mixed NULL and error pointers work.
https://staticthinking.wordpress.com/2022/08/01/mixing-error-pointers-and-null/

There are some subsystems where they use a "special" error pointer
like -ENOENT to mean "not found" instead of returning NULL but
inevitably someone is going to return -ENOENT for something else.

regards,
dan carpenter
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.