qemu-stable
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-stable] [Qemu-devel] [PATCH for-2.10 1/2] block: An empty file


From: Max Reitz
Subject: Re: [Qemu-stable] [Qemu-devel] [PATCH for-2.10 1/2] block: An empty filename counts as no filename
Date: Wed, 19 Apr 2017 15:13:36 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.0

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;
>>


Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]