Re: [Qemu-devel] [PATCH] xen_disk: cope with missing xenstore "params" node

[Peter Maydell]

On 24 June 2011 15:50,  <address@hidden> wrote:
>     /* read xenstore entries */
>     if (blkdev->params == NULL) {
>         blkdev->params = xenstore_read_be_str(&blkdev->xendev, "params");
> +        if (blkdev->params != NULL)
> +            h = strchr(blkdev->params, ':');
>         h = strchr(blkdev->params, ':');

This adds the if () statement but it looks like you forgot to remove
the strchr that is outside the if(), so this will still segfault...
(Also, coding style demands braces.)

You could also make that "char *h" local to this 'if' block.

> @@ -672,11 +674,15 @@ static int blk_init(struct XenDevice *xendev)
>         /* setup via xenbus -> create new block driver instance */
>         xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
>         blkdev->bs = bdrv_new(blkdev->dev);
> -        if (bdrv_open(blkdev->bs, blkdev->filename, qflags,
> -                      bdrv_find_whitelisted_format(blkdev->fileproto)) != 0) 
> {
> -            bdrv_delete(blkdev->bs);
> -            return -1;
> +        if (blkdev->bs) {
> +            if (bdrv_open(blkdev->bs, blkdev->filename, qflags,
> +                        bdrv_find_whitelisted_format(blkdev->fileproto)) != 
> 0) {
> +                bdrv_delete(blkdev->bs);
> +                blkdev->bs = NULL;
> +            }
>         }
> +        if (!blkdev->bs)
> +            return -1;

Doesn't this error return leak the strings allocated by
xenstore_read_be_str() ?

-- PMM