Re: [Qemu-devel] [PATCH v5 16/22] blkdebug: Make filename optional

[Kevin Wolf <kwolf@redhat.com>]

Am 13.12.2013 um 18:10 hat Max Reitz geschrieben:
> There is no problem in giving no filename; in this case, it has to be
> specified through the "image" option.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/blkdebug.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index 0c800ae..fdfc6b0 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c
> @@ -327,8 +327,10 @@ static void blkdebug_parse_filename(const char *filename, QDict *options,
>      /* Parse the blkdebug: prefix */
>      if (!strstart(filename, "blkdebug:", &filename)) {
>          /* There was no prefix; therefore, all options have to be already
> -           present in the QDict (except for the filename) */
> -        qdict_put(options, "x-image", qstring_from_str(filename));
> +           present in the QDict; if a filename is given, pass it */
> +        if (filename) {

filename can't be NULL here: For a non-NULL input, strstart() doesn't
return NULL, and for a NULL string it segfaults...

I don't think we have the segfault case here, because calling
bdrv_parse_filename() without having a filename in the first place is
quite pointless. I suspect giving no filename has been working even
before this patch.

> +            qdict_put(options, "x-image", qstring_from_str(filename));
> +        }
>          return;
>      }

Having a test case in the commit would show what your intention was (and
that the implementation doesn't do that).

Kevin