On 2021/4/9 17:55, Andy Shevchenko wrote:
> On Fri, Apr 9, 2021 at 12:42 PM Yicong Yang <yangyicong@xxxxxxxxxxxxx> wrote:
>> On 2021/4/9 17:19, Andy Shevchenko wrote:
>>> On Fri, Apr 9, 2021 at 10:22 AM Yicong Yang <yangyicong@xxxxxxxxxxxxx> wrote:
>>>> On 2021/4/8 21:09, Alexandru Ardelean wrote:
>>>>> On Thu, Apr 8, 2021 at 2:41 PM Yicong Yang <yangyicong@xxxxxxxxxxxxx> wrote:
>
> ...
>
>>> 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.
>
> Confusing here (in your case) is that:
>
> if (!FOO)
> return BAR;
>
> ...
>
> return BAR;
>
> I.e. twice returning BAR for different occasions. This makes
> additional cognitive work and decrease readability / maintainability
> of the code.
>
> I.o.w., yes, it's preferred.
>
got it. will return NULL and will check if other patches in this series
met the same condition.
thanks.
