Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10 1/2] block: An empty filename counts as no filename

[Max Reitz]

On 17.04.2017 14:51, Philippe Mathieu-Daudé wrote:
> Hi Max,
> 
> On 04/13/2017 01:06 PM, Max Reitz wrote:
>> Reproducer:
>>     $ ./qemu-img info ''
>>     qemu-img: ./block.c:1008: bdrv_open_driver: Assertion
>>         `!drv->bdrv_needs_filename || bs->filename[0]' failed.
>>     [1]    26105 abort (core dumped)  ./qemu-img info ''
>>
>> This patch fixes this to be:
>>     $ ./qemu-img info ''
>>     qemu-img: Could not open '': The 'file' block driver requires a file
>>     name
>>
>> Cc: qemu-stable <address@hidden>
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>>  block.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/block.c b/block.c
>> index 1fbbb8d606..46da908c93 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -1167,7 +1167,7 @@ static int bdrv_open_common(BlockDriverState
>> *bs, BlockBackend *file,
>>          filename = qdict_get_try_str(options, "filename");
>>      }
>>
>> -    if (drv->bdrv_needs_filename && !filename) {
>> +    if (drv->bdrv_needs_filename && (!filename || !filename[0])) {
> 
> What do you think about adding an inline function in "qemu/option.h"
> like "is_valid_[option_]filename()" to avoid this bug template?

Well, I won't do it now but it's a good idea and if I hit this issue
again (or maybe if/when I realize how much code there is that uses
constructs like this...) I'll add it.

(Although it may actually be useful with a more common name, like
string_is_empty(). Unfortunately, there isn't anything like g_strlen0(),
apparently...)

> Anyway:
> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

Thanks! (For your other reviews as well, of course.)

Max

> 
>>          error_setg(errp, "The '%s' block driver requires a file name",
>>                     drv->format_name);
>>          ret = -EINVAL;
>>

signature.asc
Description: OpenPGP digital signature