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

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

[Josua Mayer]

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?

>
>     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.

>     642 
>     643         return mux;
>     644 }
>
> This email is a free service from the Smatch-CI project [smatch.sf.net].
regards
Josua Mayer

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

[Dan Carpenter]

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