diff mbox

[01/21] block: Guard remaining unsafe blk_bs() callers

Message ID 1422300468-16216-2-git-send-email-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz Jan. 26, 2015, 7:27 p.m. UTC 
There are cases where it is probably (!) not necessary to check whether
the return value of blk_bs() is non-NULL, and those are places after
blk_new_open(). In every other place, though, there has to be some check
to make sure that the return value of blk_bs() is non-NULL before it is
used.

This patch adds that check to hopefully all of the remaining places.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 blockdev.c          | 18 ++++++++++--------
 hw/block/xen_disk.c |  4 +++-
 2 files changed, 13 insertions(+), 9 deletions(-)

Comments

Eric Blake Jan. 28, 2015, 10:14 p.m. UTC | #1 
On 01/26/2015 12:27 PM, Max Reitz wrote:
> There are cases where it is probably (!) not necessary to check whether
> the return value of blk_bs() is non-NULL, and those are places after
> blk_new_open(). In every other place, though, there has to be some check
> to make sure that the return value of blk_bs() is non-NULL before it is
> used.
> 
> This patch adds that check to hopefully all of the remaining places.

I did not audit for places you might have missed, but concur that these
needed it.

> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  blockdev.c          | 18 ++++++++++--------
>  hw/block/xen_disk.c |  4 +++-
>  2 files changed, 13 insertions(+), 9 deletions(-)


Reviewed-by: Eric Blake <eblake@redhat.com>
diff mbox

Patch

diff --git a/blockdev.c b/blockdev.c
index 8ed2fec..9c5b6ac 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -121,14 +121,16 @@  void blockdev_mark_auto_del(BlockBackend *blk)
         return;
     }
 
-    aio_context = bdrv_get_aio_context(bs);
-    aio_context_acquire(aio_context);
+    if (bs) {
+        aio_context = bdrv_get_aio_context(bs);
+        aio_context_acquire(aio_context);
 
-    if (bs->job) {
-        block_job_cancel(bs->job);
-    }
+        if (bs->job) {
+            block_job_cancel(bs->job);
+        }
 
-    aio_context_release(aio_context);
+        aio_context_release(aio_context);
+    }
 
     dinfo->auto_del = 1;
 }
@@ -228,8 +230,8 @@  bool drive_check_orphaned(void)
             dinfo->type != IF_NONE) {
             fprintf(stderr, "Warning: Orphaned drive without device: "
                     "id=%s,file=%s,if=%s,bus=%d,unit=%d\n",
-                    blk_name(blk), blk_bs(blk)->filename, if_name[dinfo->type],
-                    dinfo->bus, dinfo->unit);
+                    blk_name(blk), blk_bs(blk) ? blk_bs(blk)->filename : "",
+                    if_name[dinfo->type], dinfo->bus, dinfo->unit);
             rs = true;
         }
     }
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 1b0257c..665e706 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -933,9 +933,11 @@  static int blk_connect(struct XenDevice *xendev)
     blk_attach_dev_nofail(blkdev->blk, blkdev);
     blkdev->file_size = blk_getlength(blkdev->blk);
     if (blkdev->file_size < 0) {
+        BlockDriverState *bs = blk_bs(blkdev->blk);
+        const char *drv_name = bs ? bdrv_get_format_name(bs) : NULL;
         xen_be_printf(&blkdev->xendev, 1, "blk_getlength: %d (%s) | drv %s\n",
                       (int)blkdev->file_size, strerror(-blkdev->file_size),
-                      bdrv_get_format_name(blk_bs(blkdev->blk)) ?: "-");
+                      drv_name ?: "-");
         blkdev->file_size = 0;
     }