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
for years.

> 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?