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.
--
With Best Regards,
Andy Shevchenko
