On Thu, Jun 08, 2023 at 04:02:07AM +0000, Trevor Wu (吳文良) wrote: > On Wed, 2023-06-07 at 17:57 +0300, Dan Carpenter wrote: > > This code triggers a Smatch static checker warning and does sort of > > look like an error path. > > > > sound/soc/mediatek/mt8188/mt8188-mt6359.c:597 > > mt8188_max98390_codec_init() warn: missing error code? 'ret' > > > > However, returning 0 is intentional. Make that explicit. > > Hi Dan, > > There are several similar instances in this file. If using 'return > ret;' is not recommended, should we update all of them simultaneously? > > For instance, the value of 'ret' at the end of this function should > also be zero. > I would prefer that, yes. "return 0;" is always more clear than "return ret;" I didn't change the "return ret;" at the end of the functions because they were obviously intentional. I'm doing static analysis and I have an unpublished check which complains about the return ret at the end of the function but I'm not going to publish it because that doesn't find bugs and I'm only focused on bugs. When we have code like: ret = frob(); if (ret) return ret; if (x == 1) return ret; More often than not, that is a bug. But if it's code like: ret = frob(); if (ret) return ret; frob(); frob(); frob(); return ret; Then that's probably not a bug. regards, dan carpenter
