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

[Max Reitz]

On 13.12.2013 21:43, Kevin Wolf wrote:
> 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 <address@hidden>
> > ---
> >   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.

Right; for NULL input, that function isn't even called ("if 
(drv->bdrv_parse_filename && filename)" in bdrv_file_open()).

Max

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