Skip to contentSkip to conversation

drivers/block/cciss.c: fix check-after-use

PATCH In review Adrian Bunk 2 participants 2 messages Jul 30, 2007 – Aug 1, 2007 Block layer

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drivers/block/cciss.c: fix check-after-use

The Coverity checker spotted that we have already oops'ed if "disk" was NULL.

Since "disk" being NULL seems impossible at this point this patch removes the NULL check.

---

 drivers/block/cciss.c |   56 ++++++++++++++++++++----------------------
 1 file changed, 27 insertions(+), 29 deletions(-)
drivers/block/cciss.c renamed from drivers/block/cciss.c.old +27 -29
@@ -1569,66 +1569,64 @@ static int deregister_disk(struct gendis
 	ctlr_info_t *h = get_host(disk);
 
 	if (!capable(CAP_SYS_RAWIO))
 		return -EPERM;
 
 	/* make sure logical volume is NOT is use */
 	if (clear_all || (h->gendisk[0] == disk)) {
 		if (drv->usage_count > 1)
 			return -EBUSY;
 	} else if (drv->usage_count > 0)
 		return -EBUSY;
 
 	/* invalidate the devices and deregister the disk.  If it is disk
 	 * zero do not deregister it but just zero out it's values.  This
 	 * allows us to delete disk zero but keep the controller registered.
 	 */
 	if (h->gendisk[0] != disk) {
-		if (disk) {
-			struct request_queue *q = disk->queue;
-			if (disk->flags & GENHD_FL_UP)
-				del_gendisk(disk);
-			if (q) {
-				blk_cleanup_queue(q);
-				/* Set drv->queue to NULL so that we do not try
-				 * to call blk_start_queue on this queue in the
-				 * interrupt handler
-				 */
-				drv->queue = NULL;
-			}
-			/* If clear_all is set then we are deleting the logical
-			 * drive, not just refreshing its info.  For drives
-			 * other than disk 0 we will call put_disk.  We do not
-			 * do this for disk 0 as we need it to be able to
-			 * configure the controller.
+		struct request_queue *q = disk->queue;
+		if (disk->flags & GENHD_FL_UP)
+			del_gendisk(disk);
+		if (q) {
+			blk_cleanup_queue(q);
+			/* Set drv->queue to NULL so that we do not try
+			 * to call blk_start_queue on this queue in the
+			 * interrupt handler
+			 */
+			drv->queue = NULL;
+		}
+		/* If clear_all is set then we are deleting the logical
+		 * drive, not just refreshing its info.  For drives
+		 * other than disk 0 we will call put_disk.  We do not
+		 * do this for disk 0 as we need it to be able to
+		 * configure the controller.
+		*/
+		if (clear_all){
+			/* This isn't pretty, but we need to find the
+			 * disk in our array and NULL our the pointer.
+			 * This is so that we will call alloc_disk if
+			 * this index is used again later.
 			*/
-			if (clear_all){
-				/* This isn't pretty, but we need to find the
-				 * disk in our array and NULL our the pointer.
-				 * This is so that we will call alloc_disk if
-				 * this index is used again later.
-				*/
-				for (i=0; i < CISS_MAX_LUN; i++){
-					if(h->gendisk[i] == disk){
-						h->gendisk[i] = NULL;
-						break;
-					}
+			for (i=0; i < CISS_MAX_LUN; i++){
+				if(h->gendisk[i] == disk){
+					h->gendisk[i] = NULL;
+					break;
 				}
-				put_disk(disk);
 			}
+			put_disk(disk);
 		}
 	} else {
 		set_capacity(disk, 0);
 	}
 
 	--h->num_luns;
 	/* zero out the disk size info */
 	drv->nr_blocks = 0;
 	drv->block_size = 0;
 	drv->heads = 0;
 	drv->sectors = 0;
 	drv->cylinders = 0;
 	drv->raid_level = -1;	/* This can be used as a flag variable to
 				 * indicate that this element of the drive
 				 * array is free.
 				 */
 
Acked-by on v1 1/1
#1 lore raw
Quoted 9 lines

-----Original Message-----
From: Adrian Bunk [mailto:[email protected]]
Sent: Monday, July 30, 2007 5:28 PM
To: Miller, Mike (OS Dev); Jens Axboe
Cc: ISS StorageDev; [email protected]
Subject: [2.6 patch] drivers/block/cciss.c: fix check-after-use

Quoted 7 lines

The Coverity checker spotted that we have already oops'ed if "disk" was NULL.

Since "disk" being NULL seems impossible at this point this patch removes the NULL check.

Quoted 121 lines

---

 drivers/block/cciss.c |   56 
++++++++++++++++++++----------------------
 1 file changed, 27 insertions(+), 29 deletions(-)

--- linux-2.6.23-rc1-mm1/drivers/block/cciss.c.old

2007-07-30 02:27:15.000000000 +0200
+++ linux-2.6.23-rc1-mm1/drivers/block/cciss.c	
2007-07-30 02:28:28.000000000 +0200
drivers/block/cciss.c renamed from drivers/block/cciss.c.old +0 -0
@@ -1569,66 +1569,64 @@ static int deregister_disk(struct gendis
 	ctlr_info_t *h = get_host(disk);
 
 	if (!capable(CAP_SYS_RAWIO))
 		return -EPERM;
 
 	/* make sure logical volume is NOT is use */
 	if (clear_all || (h->gendisk[0] == disk)) {
 		if (drv->usage_count > 1)
 			return -EBUSY;
 	} else if (drv->usage_count > 0)
 		return -EBUSY;
 
 	/* invalidate the devices and deregister the disk.  If 

it is disk
* zero do not deregister it but just zero out it's
values. This
* allows us to delete disk zero but keep the
controller registered.

drivers/block/cciss.c renamed from drivers/block/cciss.c.old +0 -7
 	 */
 	if (h->gendisk[0] != disk) {
-		if (disk) {
-			struct request_queue *q = disk->queue;
-			if (disk->flags & GENHD_FL_UP)
-				del_gendisk(disk);
-			if (q) {
-				blk_cleanup_queue(q);
-				/* Set drv->queue to NULL so 

that we do not try

(quoted diff) +0 -1
-				 * to call blk_start_queue on 

this queue in the

drivers/block/cciss.c renamed from drivers/block/cciss.c.old +0 -5
-				 * interrupt handler
-				 */
-				drv->queue = NULL;
-			}
-			/* If clear_all is set then we are 

deleting the logical

(quoted diff) +0 -2
-			 * drive, not just refreshing its info. 
 For drives
-			 * other than disk 0 we will call 

put_disk. We do not

(quoted diff) +0 -1
-			 * do this for disk 0 as we need it to 

be able to

drivers/block/cciss.c renamed from drivers/block/cciss.c.old +22 -3
-			 * configure the controller.
+		struct request_queue *q = disk->queue;
+		if (disk->flags & GENHD_FL_UP)
+			del_gendisk(disk);
+		if (q) {
+			blk_cleanup_queue(q);
+			/* Set drv->queue to NULL so that we do not try
+			 * to call blk_start_queue on this queue in the
+			 * interrupt handler
+			 */
+			drv->queue = NULL;
+		}
+		/* If clear_all is set then we are deleting the logical
+		 * drive, not just refreshing its info.  For drives
+		 * other than disk 0 we will call put_disk.  We do not
+		 * do this for disk 0 as we need it to be able to
+		 * configure the controller.
+		*/
+		if (clear_all){
+			/* This isn't pretty, but we need to find the
+			 * disk in our array and NULL our the pointer.
+			 * This is so that we will call alloc_disk if
+			 * this index is used again later.
 			*/
-			if (clear_all){
-				/* This isn't pretty, but we 

need to find the

(quoted diff) +0 -1
-				 * disk in our array and NULL 

our the pointer.

(quoted diff) +0 -1
-				 * This is so that we will call 

alloc_disk if

drivers/block/cciss.c renamed from drivers/block/cciss.c.old +5 -8
-				 * this index is used again later.
-				*/
-				for (i=0; i < CISS_MAX_LUN; i++){
-					if(h->gendisk[i] == disk){
-						h->gendisk[i] = NULL;
-						break;
-					}
+			for (i=0; i < CISS_MAX_LUN; i++){
+				if(h->gendisk[i] == disk){
+					h->gendisk[i] = NULL;
+					break;
 				}
-				put_disk(disk);
 			}
+			put_disk(disk);
 		}
 	} else {
 		set_capacity(disk, 0);
 	}
 
 	--h->num_luns;
 	/* zero out the disk size info */
 	drv->nr_blocks = 0;
 	drv->block_size = 0;
 	drv->heads = 0;
 	drv->sectors = 0;
 	drv->cylinders = 0;
 	drv->raid_level = -1;	/* This can be used as a flag 

variable to
* indicate that this element
of the drive

drivers/block/cciss.c renamed from drivers/block/cciss.c.old +0 -0
 				 * array is free.
 				 */