* Re: [bug report] mux: Add helper functions for getting optional and selected mux-state
2026-04-19 10:16 ` [bug report] mux: Add helper functions for getting optional and selected mux-state Josua Mayer
@ 2026-04-20 14:45 ` Dan Carpenter
0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2026-04-20 14:45 UTC (permalink / raw)
To: Josua Mayer
Cc: kernel-janitors@vger.kernel.org, Peter Rosin, kees@kernel.org,
thorsten.blum@linux.dev, ulfh@kernel.org, Wolfram Sang,
linux-kernel@vger.kernel.org
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
^ permalink raw reply [flat|nested] 2+ messages in thread