> -----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; linux-kernel@vger.kernel.org
> Subject: [2.6 patch] 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.
> 
> Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]>

Acked-by: Mike Miller <[EMAIL PROTECTED]>

> 
> ---
> 
>  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
> @@ -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.
>                                */
>  
> 
> 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to