|
From: | John Snow |
Subject: | Re: [Qemu-devel] [PATCH 1/3] qemu-img: fix img_commit() error return value |
Date: | Tue, 26 Aug 2014 16:17:46 -0400 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0 |
On 08/26/2014 03:31 PM, Eric Blake wrote:
On 08/26/2014 12:17 PM, Stefan Hajnoczi wrote:The img_commit() return value is a process exit code. Use 1 for failure instead of -1. The other failure paths in this function already use 1. Signed-off-by: Stefan Hajnoczi <address@hidden> --- qemu-img.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-img.c b/qemu-img.c index c843420..dc3adb5 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -771,7 +771,7 @@ static int img_commit(int argc, char **argv) ret = bdrv_parse_cache_flags(cache, &flags); if (ret < 0) { error_report("Invalid cache option: %s", cache); - return -1; + return 1;Nothing against this patch (you're consistent with the surrounding code, and most of qemu for that matter), but it highlights why I'm a fan of 'return EXIT_FAILURE' instead of 'return 1' in functions that return an exit status, because that makes it a lot more obvious _why_ I'm returning a non-negative number to represent failure.
"Hey, good point." address@hidden ~/s/qemu> git grep 'return 1;' | wc -l 842 "oh."This patch is probably fine -- is there some larger scheme we want to cook up within the style guide for advocating things like return code and error reporting standards?
It seems to me like the preferred style for errors and returns has changed several times and there's not a good concrete rule (written) at the moment. It might be worth touching the CODING_GUIDE and/or HACKING files with recommendations, if we can agree on some consistent set of rules, like getting rid of error_set(...) and not using positive, un-named integers to represent errors.
-- —js
[Prev in Thread] | Current Thread | [Next in Thread] |