[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] xen_disk: cope with missing xenstore "params" n
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [PATCH] xen_disk: cope with missing xenstore "params" node |
Date: |
Fri, 11 Feb 2011 12:59:53 +0000 |
User-agent: |
Alpine 2.00 (DEB 1167 2008-08-23) |
On Fri, 11 Feb 2011, Kevin Wolf wrote:
> Am 11.02.2011 13:38, schrieb Stefano Stabellini:
> > When disk is a cdrom and the drive is empty the "params" node in
> > xenstore might be missing completely: cope with it instead of
> > segfaulting.
> >
> > Signed-off-by: Stefano Stabellini <address@hidden>
> >
> >
> > diff --git a/hw/xen_disk.c b/hw/xen_disk.c
> > index 134ac33..e553c4c 100644
> > --- a/hw/xen_disk.c
> > +++ b/hw/xen_disk.c
> > @@ -577,12 +577,13 @@ static int blk_init(struct XenDevice *xendev)
> > {
> > struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev,
> > xendev);
> > int index, qflags, have_barriers, info = 0;
> > - char *h;
> > + char *h = NULL;
> >
> > /* read xenstore entries */
> > if (blkdev->params == NULL) {
> > blkdev->params = xenstore_read_be_str(&blkdev->xendev, "params");
> > - h = strchr(blkdev->params, ':');
> > + if (blkdev->params != NULL)
> > + h = strchr(blkdev->params, ':');
>
> The coding style requires braces here.
>
Good point, I'll do.
> > if (h != NULL) {
> > blkdev->fileproto = blkdev->params;
> > blkdev->filename = h+1;
>
> Let me add some more context:
>
> if (h != NULL) {
> blkdev->fileproto = blkdev->params;
> blkdev->filename = h+1;
> *h = 0;
> } else {
> blkdev->fileproto = "<unset>";
> blkdev->filename = blkdev->params;
> }
>
> So in the NULL case we now have blkdev->filename = NULL. Doesn't this
> just move the crash a few lines downwards when bdrv_open() tries to use
> NULL as its filename?
There is a check on blkdev->params being NULL few lines after so we just
return.
Maybe an explicit return -1 like in the appended patch here would be
better?
diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index 134ac33..fc0de14 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -582,6 +582,9 @@ static int blk_init(struct XenDevice *xendev)
/* read xenstore entries */
if (blkdev->params == NULL) {
blkdev->params = xenstore_read_be_str(&blkdev->xendev, "params");
+ if (blkdev->params == NULL) {
+ return -1;
+ }
h = strchr(blkdev->params, ':');
if (h != NULL) {
blkdev->fileproto = blkdev->params;