On Fri, May 19, 2023 at 01:58:10PM +0300, Dan Carpenter wrote: > This is unrelated but I was looking through the driver and I notice > a bunch of code doing: > > grep 'return ret ?' drivers/firmware/ -R > > return ret ? : res.result[0]; > > "ret" here is a kernel error code, and res.result[0] is a firmware > error code. Mixing error codes is a dangerous thing. I was reviewing > some of the callers and the firmware error code gets passed quite far > back into the kernel to where we would only expect kernel error codes. > > Presumably the firmware is returning positive error codes? To be honest, > I am just guessing. It's better to convert custom error codes to kernel > error codes as soon as possible. I am just guessing. Sukrut, do you > think you could take a look? If the callers do not differentiate > between negative kernel error codes and positive custom error codes then > probably just do: > > if (res.result[0]) > ret = -EIO; // -EINVAL? > return ret; > Thanks, Dan, for sharing your findings. Yes, sure, I will take a look. Regards, Sukrut Bellary > Also there are a couple places which do: > > return ret ? false : !!res.result[0]; > > Here true means success and false means failure. So the !! converts > a firmware error code to true when it should be false so that's a bug. > Quadruple negatives are confusing... It should be: > > if (ret || res.result[0]) > return false; > return true; > > regards, > dan carpenter >
