On 4/27/20 5:00 AM, Max Reitz wrote:
On 24.04.20 21:09, Eric Blake wrote:
There are several callers that need to create a new block backend from
an existing BDS; make the task slightly easier with a common helper
routine.
Suggested-by: Max Reitz <mre...@redhat.com>
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
+++ b/block/crypto.c
@@ -256,16 +256,14 @@ static int
block_crypto_co_create_generic(BlockDriverState *bs,
PreallocMode prealloc,
Error **errp)
{
- int ret;
+ int ret = -EPERM;
I’m not sure I’m a fan of this, because I feel like it makes the code
harder to read, due to having to look in three places (here, around the
blk_new_with_bs() call, and under the cleanup label) instead of in two
(not here) to verify that the error handling code is correct.
There’s also the fact that this is not really a default return value,
but one very specific error code for if one very specific function call
fails.
I suppose it comes down to whether one considers LoC a complexity
problem. (I don’t, necessarily.)
(Also I realize it seems rather common in the kernel to set error return
variables before the function call, but I think the more common pattern
in qemu is to set it in the error path.)
I'm fine with either style. Setting it up front is handy if that
particular error makes a good default, but in many of the functions I
touched, we were returning a variety of errors (-EIO, -EINVAL, -EPERM,
etc) such that there was no good default, and thus no reason to set a
default up front. Is this something that would go through your tree,
and if so, are you okay making that tweak, or do I need to send v4?
But it does look like you got all cases covered this time.
Good to hear.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
sheepdog mailing list
sheepdog@lists.wpkg.org
https://lists.wpkg.org/mailman/listinfo/sheepdog
