Re: [PATCH v2 4/7] remoteproc: core: Use cleanup.h macros to simplify lock handling

Dan Carpenter
2025-10-14T11:32:00Z

On Tue, Oct 14, 2025 at 06:45:11PM +0800, Peng Fan wrote:
> Hi Dan,
>
> I am not sure, Is this false alarm?
>
> On Tue, Oct 14, 2025 at 11:39:41AM +0300, Dan Carpenter wrote:
> >Hi Peng,
> >
> >
> >vim +/ret +1841 drivers/remoteproc/remoteproc_core.c
> >
> >70b85ef83ce3523 Fernando Guzman Lugo 2012-08-30 1829 int rproc_trigger_recovery(struct rproc *rproc)
> >70b85ef83ce3523 Fernando Guzman Lugo 2012-08-30 1830 {
> >7e83cab824a8670 Sarangdhar Joshi 2017-05-26 1831 struct device *dev = &rproc->dev;
> >7e83cab824a8670 Sarangdhar Joshi 2017-05-26 1832 int ret;
> >7e83cab824a8670 Sarangdhar Joshi 2017-05-26 1833
> >c42baf6f84c7694 Peng Fan 2025-10-10 1834 ACQUIRE(mutex_intr, lock)(&rproc->lock);
> >c42baf6f84c7694 Peng Fan 2025-10-10 1835 ret = ACQUIRE_ERR(mutex_intr, &lock);
> >7e83cab824a8670 Sarangdhar Joshi 2017-05-26 1836 if (ret)
> >7e83cab824a8670 Sarangdhar Joshi 2017-05-26 1837 return ret;
> >7e83cab824a8670 Sarangdhar Joshi 2017-05-26 1838
> >0b145574b6cd2b3 Alex Elder 2020-02-28 1839 /* State could have changed before we got the mutex */
> >0b145574b6cd2b3 Alex Elder 2020-02-28 1840 if (rproc->state != RPROC_CRASHED)
> >c42baf6f84c7694 Peng Fan 2025-10-10 @1841 return ret;
> >
> >Please change this to either "return 0;" or "return -ERRORCODE;"
>
> ACQUIRE_ERR should already returns 0. This change does not change the
> assignment to ret as my understanding. Please help to see if this is false
> alarm or I miss something?
>

I guess if this was already merged then it's fine. But "return ret" looks
like an error path where "return 0;" is obvious. This code will always
trigger a Smatch warning, and I always tell people that old code has been
reviewed so all the warnings are false positives, still someone will
eventually change this to "return -EINVAL;" because it looks so much like
a mistake.

regards,
dan carpenter