[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] Re: [Qemu-devel] [PATCH] xen_disk: cope with missing xenstore "params" node
On Fri, 24 Jun 2011, Peter Maydell wrote: > On 24 June 2011 15:50, <stefano.stabellini@xxxxxxxxxxxxx> 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. Thank you very much for the review, I'll make the changes. > > > @@ -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() ? Another very good point, I'll introduce an out_error label and free everything there. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |