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<kw...@redhat.com>
>>> Signed-off-by: Stefan Weil<w...@mail.berlios.de>
>>> ---
>>> 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
