From: SF Markus Elfring <elfring@users.sourceforge.net>
To: "Christian Bornträger" <borntraeger@de.ibm.com>,
virtualization@lists.linux-foundation.org,
"Michael S. Tsirkin" <mst@redhat.com>
Cc: Julia Lawall <julia.lawall@lip6.fr>,
kernel-janitors@vger.kernel.org,
LKML <linux-kernel@vger.kernel.org>,
Minfei Huang <mnghuan@gmail.com>,
linux-doc@vger.kernel.org
Subject: Re: virtio_blk: Less function calls in init_vq() after error detection
Date: Tue, 13 Sep 2016 16:33:03 +0200 [thread overview]
Message-ID: <55fbd4d0-3fb3-0334-6545-cb4da0d12edf@users.sourceforge.net> (raw)
In-Reply-To: <e918e655-cd86-c3c8-d911-9dfc03b03e19@de.ibm.com>
>> 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?
Regards,
Markus
next prev parent reply other threads:[~2016-09-13 14:33 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <566ABCD9.1060404@users.sourceforge.net>
2016-09-13 12:10 ` [PATCH 0/4] block-virtio: Fine-tuning for two function implementations SF Markus Elfring
2016-09-13 12:12 ` [PATCH 1/4] virtio_blk: Use kmalloc_array() in init_vq() SF Markus Elfring
2016-09-13 12:13 ` [PATCH 2/4] virtio_blk: Less function calls in init_vq() after error detection SF Markus Elfring
2016-09-13 12:14 ` [PATCH 3/4] virtio_blk: Delete an unnecessary initialisation in init_vq() SF Markus Elfring
2016-09-13 12:15 ` [PATCH 4/4] virtio_blk: Rename a jump label in virtblk_get_id() SF Markus Elfring
[not found] ` <f56845a8-03c6-d3f7-6091-99dba9835780@users.sourceforge.net>
2016-09-13 12:54 ` [PATCH 2/4] virtio_blk: Less function calls in init_vq() after error detection Christian Borntraeger
[not found] ` <e918e655-cd86-c3c8-d911-9dfc03b03e19@de.ibm.com>
2016-09-13 14:33 ` SF Markus Elfring [this message]
2016-09-13 17:30 ` SF Markus Elfring
[not found] ` <7da823eb-939c-9ee6-32bf-db296e6a96f6@users.sourceforge.net>
2016-09-13 18:24 ` Christian Borntraeger
2016-09-14 6:56 ` SF Markus Elfring
2016-09-14 8:10 ` Cornelia Huck
[not found] ` <20160914101009.6abef9f0.cornelia.huck@de.ibm.com>
2016-09-14 9:09 ` virtio_blk: Clarification for communication difficulties? SF Markus Elfring
[not found] ` <a1642c2a-c013-2dec-29fb-1748a52e1c24@users.sourceforge.net>
2016-10-03 9:20 ` Stefan Hajnoczi
[not found] ` <CAJSP0QV_V-aEvdE76PnOH6TNJPJCuf+6N7SkySDAnrhbNNhv3w@mail.gmail.com>
2016-10-03 12:00 ` SF Markus Elfring
[not found] ` <52a07fc8-21a0-8f98-fa9d-5751fbf95afa@users.sourceforge.net>
2016-10-03 9:09 ` [PATCH 3/4] virtio_blk: Delete an unnecessary initialisation in init_vq() Stefan Hajnoczi
[not found] ` <7a8dd874-3700-1445-2143-2a604cd043ab@users.sourceforge.net>
2016-10-03 9:11 ` [PATCH 1/4] virtio_blk: Use kmalloc_array() " Stefan Hajnoczi
[not found] ` <a303f7a6-c675-5228-99bd-a03c9e9252e9@users.sourceforge.net>
2016-10-03 9:07 ` [PATCH 4/4] virtio_blk: Rename a jump label in virtblk_get_id() Stefan Hajnoczi
[not found] ` <CAJSP0QV2WUJPqeiSKcWiXWk+AoJ2MGo2zG4=JQ2tfpTprAyV=g@mail.gmail.com>
2016-10-03 12:12 ` SF Markus Elfring
2016-10-09 23:30 ` [PATCH 4/4] " Michael S. Tsirkin
2016-10-10 8:18 ` SF Markus Elfring
2016-09-14 13:56 ` [PATCH 00/11] virtio-console: Fine-tuning for 14 function implementations SF Markus Elfring
2016-09-14 14:00 ` [PATCH 01/11] virtio_console: Use kmalloc_array() in init_vqs() SF Markus Elfring
2016-09-14 14:01 ` [PATCH 02/11] virtio_console: Less function calls in init_vqs() after error detection SF Markus Elfring
2016-09-21 12:10 ` Amit Shah
2016-09-21 13:06 ` SF Markus Elfring
2016-09-14 14:02 ` [PATCH 03/11] virtio_console: Rename a jump label in init() SF Markus Elfring
2016-09-14 14:03 ` [PATCH 04/11] virtio_console: Rename jump labels in virtcons_probe() SF Markus Elfring
2016-09-14 14:04 ` [PATCH 05/11] virtio_console: Rename jump labels in add_port() SF Markus Elfring
2016-09-14 14:05 ` [PATCH 06/11] virtio_console: Rename a jump label in port_fops_open() SF Markus Elfring
2016-09-14 14:06 ` [PATCH 07/11] virtio_console: Rename a jump label in port_fops_splice_write() SF Markus Elfring
2016-09-14 14:07 ` [PATCH 08/11] virtio_console: Rename jump labels in port_fops_write() SF Markus Elfring
2016-09-14 14:08 ` [PATCH 09/11] virtio_console: Rename a jump label in __send_to_port() SF Markus Elfring
2016-09-14 14:09 ` [PATCH 10/11] virtio_console: Rename jump labels in alloc_buf() SF Markus Elfring
2016-09-14 14:10 ` [PATCH 11/11] virtio_console: Rename a jump label in five functions SF Markus Elfring
2017-08-06 10:56 ` [PATCH 00/11] virtio-console: Fine-tuning for 14 function implementations SF Markus Elfring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55fbd4d0-3fb3-0334-6545-cb4da0d12edf@users.sourceforge.net \
--to=elfring@users.sourceforge.net \
--cc=borntraeger@de.ibm.com \
--cc=julia.lawall@lip6.fr \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mnghuan@gmail.com \
--cc=mst@redhat.com \
--cc=virtualization@lists.linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).