Re: [PATCH v4 1/7] iio: Use an early return in iio_device_alloc to simplify code.

From: Ardelean, Alexandru
Date: Sun Apr 26 2020 - 03:28:26 EST


On Fri, 2020-04-24 at 07:56 +0300, Alexandru Ardelean wrote:
> From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>
> Noticed whilst reviewing Alexandru's patch to the same function.
> If we simply flip the logic and return NULL immediately after memory
> allocation failure we reduce the indent of the following block and
> end up with more 'idiomatic' kernel code.
>

Will need to re-update the series.
Since I wasn't sure how the discussion on the initial version would unfold, I
made this a bit modified.
I think I forgot to mention this initially.

In any case.
Will re-base a new series, without this patch.


> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> Cc: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
> Reviewed-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
> ---
> drivers/iio/industrialio-core.c | 46 ++++++++++++++++-----------------
> 1 file changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index f4daf19f2a3b..3c97090c2ab9 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1492,7 +1492,7 @@ struct device_type iio_device_type = {
> **/
> struct iio_dev *iio_device_alloc(int sizeof_priv)
> {
> - struct iio_dev *dev;
> + struct iio_dev *indio_dev;
> size_t alloc_size;
>
> alloc_size = sizeof(struct iio_dev);
> @@ -1503,30 +1503,30 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
> /* ensure 32-byte alignment of whole construct ? */
> alloc_size += IIO_ALIGN - 1;
>
> - dev = kzalloc(alloc_size, GFP_KERNEL);
> -
> - if (dev) {
> - dev->dev.groups = dev->groups;
> - dev->dev.type = &iio_device_type;
> - dev->dev.bus = &iio_bus_type;
> - device_initialize(&dev->dev);
> - dev_set_drvdata(&dev->dev, (void *)dev);
> - mutex_init(&dev->mlock);
> - mutex_init(&dev->info_exist_lock);
> - INIT_LIST_HEAD(&dev->channel_attr_list);
> -
> - dev->id = ida_simple_get(&iio_ida, 0, 0, GFP_KERNEL);
> - if (dev->id < 0) {
> - /* cannot use a dev_err as the name isn't available */
> - pr_err("failed to get device id\n");
> - kfree(dev);
> - return NULL;
> - }
> - dev_set_name(&dev->dev, "iio:device%d", dev->id);
> - INIT_LIST_HEAD(&dev->buffer_list);
> + indio_dev = kzalloc(alloc_size, GFP_KERNEL);
> + if (!indio_dev)
> + return NULL;
> +
> + indio_dev->id = ida_simple_get(&iio_ida, 0, 0, GFP_KERNEL);
> + if (indio_dev->id < 0) {
> + /* cannot use a dev_err as the name isn't available */
> + pr_err("failed to get device id\n");
> + kfree(indio_dev);
> + return NULL;
> }
>
> - return dev;
> + dev_set_name(&indio_dev->dev, "iio:device%d", indio_dev->id);
> + indio_dev->dev.groups = indio_dev->groups;
> + indio_dev->dev.type = &iio_device_type;
> + indio_dev->dev.bus = &iio_bus_type;
> + device_initialize(&indio_dev->dev);
> + dev_set_drvdata(&indio_dev->dev, (void *)indio_dev);
> + mutex_init(&indio_dev->mlock);
> + mutex_init(&indio_dev->info_exist_lock);
> + INIT_LIST_HEAD(&indio_dev->channel_attr_list);
> + INIT_LIST_HEAD(&indio_dev->buffer_list);
> +
> + return indio_dev;
> }
> EXPORT_SYMBOL(iio_device_alloc);
>