Re: [PATCH] md: raid0: Simplify raid0_run().

SandeepKsinha
2009-05-14T11:43:18Z

Hi Andre,

On Thu, May 14, 2009 at 4:13 PM, Andre Noll <maan@systemlinux.org> wrote:
> Get rid of the local variable "conf" and of an unnecessary cast.
>
> Signed-off-by: Andre Noll <maan@systemlinux.org>
> ---
>  drivers/md/raid0.c |   20 +++++++-------------
>  1 files changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
> index 5ff6290..36b747a 100644
> --- a/drivers/md/raid0.c
> +++ b/drivers/md/raid0.c
> @@ -261,7 +261,6 @@ static sector_t raid0_size(mddev_t *mddev, sector_t sectors, int raid_disks)
>
>  static int raid0_run(mddev_t *mddev)
>  {
> -       raid0_conf_t *conf;
>        int ret;
>
>        if (mddev->chunk_size == 0) {
> @@ -276,14 +275,15 @@ static int raid0_run(mddev_t *mddev)
>        blk_queue_segment_boundary(mddev->queue, (mddev->chunk_size>>1) - 1);
>        mddev->queue->queue_lock = &mddev->queue->__queue_lock;
>
> -       conf = kmalloc(sizeof (raid0_conf_t), GFP_KERNEL);
> -       if (!conf)
> +       mddev->private = kmalloc(sizeof(raid0_conf_t), GFP_KERNEL);
> +       if (!mddev->private)
>                return -ENOMEM;
> -       mddev->private = (void *)conf;
> -
>        ret = create_strip_zones(mddev);
> -       if (ret < 0)
> -               goto out_free_conf;
> +       if (ret < 0) {
> +               kfree(mddev->private);
> +               mddev->private = NULL;
> +               return ret;
> +       }
>

I believe the use of goto statements keep the code more structured,
especially for code cleanup in case of failures.
Its better that you revert back to the goto's so that later if a new
piece of code is added and there would be cases of failure to handle,
you will have a common cleanup routine.

I think Neil can comment better on this one.

>        /* calculate array device size */
>        md_set_array_sectors(mddev, raid0_size(mddev, 0, 0));
> @@ -305,14 +305,8 @@ static int raid0_run(mddev_t *mddev)
>                        mddev->queue->backing_dev_info.ra_pages = 2* stripe;
>        }
>
> -
>        blk_queue_merge_bvec(mddev->queue, raid0_mergeable_bvec);
>        return 0;
> -
> -out_free_conf:
> -       kfree(conf);
> -       mddev->private = NULL;
> -       return ret;
>  }
>
>  static int raid0_stop (mddev_t *mddev)
> --
> 1.5.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
Regards,
Sandeep.

“To learn is to change. Education is a process that changes the learner.”
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html