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