| Message ID | 20170413160625.27719-2-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 <qemu-stable@nongnu.org> > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > 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? Anyway: Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > error_setg(errp, "The '%s' block driver requires a file name", > drv->format_name); > ret = -EINVAL; >
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 <qemu-stable@nongnu.org> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> 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é <f4bug@amsat.org> 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; >>
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])) { error_setg(errp, "The '%s' block driver requires a file name", drv->format_name); ret = -EINVAL;
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 <qemu-stable@nongnu.org> Signed-off-by: Max Reitz <mreitz@redhat.com> --- block.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)