|
From: | Stefan Weil |
Subject: | [Qemu-devel] Re: [PATCH] block: Use error codes from lower levels for error message |
Date: | Mon, 19 Jul 2010 22:55:17 +0200 |
User-agent: | Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.10) Gecko/20100620 Iceowl/1.0b1 Icedove/3.0.5 |
Am 19.07.2010 14:26, schrieb Kevin Wolf:
Am 18.07.2010 21:42, schrieb Stefan Weil:"No such file or directory" is a misleading error message when a user tries to open a file with wrong permissions. Cc: Kevin Wolf<address@hidden> Signed-off-by: Stefan Weil<address@hidden> --- block.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/block.c b/block.c index f837876..2f80540 100644 --- a/block.c +++ b/block.c @@ -330,16 +330,20 @@ BlockDriver *bdrv_find_protocol(const char *filename) return NULL; } -static BlockDriver *find_image_format(const char *filename) +static BlockDriver *find_image_format(const char *filename, int *error)Wouldn't it be a more natural interface to return an 0/-errno int and pass the BlockDriver* by reference? I think we already have some function that work this way in the block code, but I can't remember any that get an int *error.
... nor did I find a function which takes a BlockDriver**. But if you prefer it like that, I can send a new version of the patch.
{ int ret, score, score_max; BlockDriver *drv1, *drv; uint8_t buf[2048]; BlockDriverState *bs; + *error = -ENOENT;Why -ENOENT is the default would be clearer if you moved it down next to the drv = NULL before the loop that searches for the driver.
What about the "return bdrv_find_format" lines? They need a default value, too. And I did not want to change too much because I cannot run a complete test for all cases. So setting *error at the beginning should be the safest modification.
Apart from these minor nitpicks it looks good. Kevin
Thanks. Stefan
[Prev in Thread] | Current Thread | [Next in Thread] |