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 <[email protected]> >> Signed-off-by: Max Reitz <[email protected]> >> --- >> 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é <[email protected]> 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
