From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=52345 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ob7V5-0005GU-8f for qemu-devel@nongnu.org; Tue, 20 Jul 2010 03:44:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1Ob7V3-0006ZH-KL for qemu-devel@nongnu.org; Tue, 20 Jul 2010 03:44:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:27809) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Ob7V3-0006Ym-Ce for qemu-devel@nongnu.org; Tue, 20 Jul 2010 03:44:25 -0400 Message-ID: <4C4553D5.7090104@redhat.com> Date: Tue, 20 Jul 2010 09:44:21 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1279482150-19385-1-git-send-email-weil@mail.berlios.de> <4C44447D.9020405@redhat.com> <4C44BBB5.6070600@mail.berlios.de> In-Reply-To: <4C44BBB5.6070600@mail.berlios.de> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH] block: Use error codes from lower levels for error message List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: QEMU Developers Am 19.07.2010 22:55, schrieb Stefan Weil: > 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 >>> Signed-off-by: Stefan Weil >>> --- >>> 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. Yes, I would prefer it like that. If you don't mind, please send a new version. >> >>> { >>> 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. You mean the return bdrv_find_format("raw")? Shouldn't ever fail unless someone was crazy enough to compile raw out, but you're right. Better leave it as it is. Kevin