linux-kernel - Re: [PATCH v4 1/7] iio: Use an early return in iio_device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7373dddfb7b3fca6808bd9995ebdd57201983a73.camel@analog.com>
Date:   Sun, 26 Apr 2020 07:28:05 +0000
From:   "Ardelean, Alexandru" <alexandru.Ardelean@...log.com>
To:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>
CC:     "jic23@...nel.org" <jic23@...nel.org>,
        "Jonathan.Cameron@...wei.com" <Jonathan.Cameron@...wei.com>,
        "lars@...afoo.de" <lars@...afoo.de>,
        "pmeerw@...erw.net" <pmeerw@...erw.net>
Subject: Re: [PATCH v4 1/7] iio: Use an early return in iio_device_alloc to
 simplify code.

On Fri, 2020-04-24 at 07:56 +0300, Alexandru Ardelean wrote:
> From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> 
> 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@...wei.com>
> Cc: Alexandru Ardelean <alexandru.ardelean@...log.com>
> Reviewed-by: Alexandru Ardelean <alexandru.ardelean@...log.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@...log.com>
> ---
>  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);
>