Re: [PATCH 5/7] iio: core: simplify some devm functions

From: Yicong Yang <yangyicong@hisilicon.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Alexandru Ardelean <ardeleanalex@gmail.com>,
	Jonathan Cameron <jic23@kernel.org>,
	linux-iio <linux-iio@vger.kernel.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	"Hennerich, Michael" <Michael.Hennerich@analog.com>,
	"Peter Meerwald-Stadler" <pmeerw@pmeerw.net>,
	<prime.zeng@huawei.com>, Tian Tao <tiantao6@hisilicon.com>
Subject: Re: [PATCH 5/7] iio: core: simplify some devm functions
Date: Fri, 9 Apr 2021 17:41:38 +0800	[thread overview]
Message-ID: <94bbe851-1f09-7099-38ac-2b6fcb056349@hisilicon.com> (raw)
In-Reply-To: <CAHp75Ve_JwGeruZeZnVk3pXXmETVERQTdR2+MSPX=Y75+nQLcw@mail.gmail.com>

On 2021/4/9 17:19, Andy Shevchenko wrote:
> On Fri, Apr 9, 2021 at 10:22 AM Yicong Yang <yangyicong@hisilicon.com> wrote:
>> On 2021/4/8 21:09, Alexandru Ardelean wrote:
>>> On Thu, Apr 8, 2021 at 2:41 PM Yicong Yang <yangyicong@hisilicon.com> wrote:
> 
> ...
> 
>>>> -       if (!ptr)
>>>> -               return NULL;
> 
>>>> +       if (!iio_dev)
>>>> +               return iio_dev;
>>>
>>> This is correct.
>>> But the preference is usually:
>>>         if (!iio_dev)
>>>             return NULL;
>>>
>>
>> since it returned iio_dev when failure before, so i just keep as is
> 
> Actually Alexandru has a good point. Compare the pieces I left above.
> 

a little different from my perspective when I did this.

previously:

-       ptr = devres_alloc(devm_iio_device_release, sizeof(*ptr),
-                          GFP_KERNEL);
-       if (!ptr)
-               return NULL;

if devres_alloc() failure we return NULL.

        iio_dev = iio_device_alloc(parent, sizeof_priv);
-       if (iio_dev) {
-               *ptr = iio_dev;
-               devres_add(parent, ptr);
-       } else {
-               devres_free(ptr);
-       }

        return iio_dev;

if iio_device_alloc() failed, we return what it returns.

now we remove devres_alloc() and stay iio_device_alloc(), so I just
keep to return iio_dev.

but actually iio_device_alloc() will return NULL at failure. :)
so return NULL here is definitely correct. i'll change to that if
it's recommended.

thanks,
Yicong