[PATCH] staging: sunxi: cedrus: centralize cedrus_open exit

Dan Carpenter dan.carpenter at oracle.com
Mon Apr 25 02:36:36 PDT 2022


On Sat, Apr 23, 2022 at 02:01:11PM -0400, Ian Cowan wrote:
> Refactor the cedrus_open() function so that there is only one exit to
> the function instead of 2. This prevents a future change from preventing
> the mutex from being unlocked after a successful exit.

Future proofing code is a complicated thing because the future is hard
to predict.

But one rule is that normally the success path gets tested.  No one is
going to forget to drop the lock on the success path.  Or if we do forget,
it likely will get caught quickly and it will be easy to bisect.

Generally it's best to keep the success path and the failure path as
separate as possible.  Avoid interleaving.  It's more readable.  If
there is no cleanup on failure then it's okay to have:

unlock:
	unlock(foo);
	return ret;

But once you have cleanup then put that in a separate cleanup block.

As well Smatch can recognize a typical kernel cleanup block so if you
write it in a weird way then you're disabling the static checker warning
for missing error codes.  I probably am going to write more checks which
are specific to cleanup blocks in the future.

regards,
dan carpenter



More information about the linux-arm-kernel mailing list