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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux