Re: virtio_blk: Less function calls in init_vq() after error detection
From: SF Markus Elfring
Date: Tue Sep 13 2016 - 10:33:35 EST
>> drivers/block/virtio_blk.c | 22 +++++++++++++++++-----
>> 1 file changed, 17 insertions(+), 5 deletions(-)
> Can't you see from this diffstat that the patch actually seems to makes
> the code more complex?
I find that the repeated usage of a bit more error handling code is almost
unavoidable if you would like to handle allocation failures more directly
as I dared to propose again here.
> In addition, please have a look at commit 347a529398e8e723338cca5d8a8ae2d9e7e93448
> virtio_blk: Fix a slient kernel panic
> which did the opposite of your patch.
This software update adjusted also the jump targets. This approach
triggered another update suggestion (in addition to improvements around
the function "kmalloc_array").
Such a software development shows different views on the implementation
for correct exception handling. I am not so "silent" on this development topic
> And in fact it fixed a bug.
I get the impression from Minfei's contribution that the statement "err = -ENOMEM;"
was added behind memory allocations.
It was also chosen to restructure this function implementation so that
the single label "out" was used there for a while.
* Is this detail worth for another look?
* How does this name selection fit to the current Linux coding style convention?
> Quite obviously multiple labels are harder to read
I do not agree agree completely to your opinion.
> and harder to get right.
These identifiers can generate their own kind of software development
challenges as usual.
> For error handling with just kfree one label is just the right thing to.
This approach can look convenient at first glance.
Does the correctness aspect need any further considerations?