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

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 9 Apr 2021 21:05:49 +0800
Yicong Yang <yangyicong@hisilicon.com> wrote:

> On 2021/4/9 17:55, Andy Shevchenko wrote:
> > On Fri, Apr 9, 2021 at 12:42 PM Yicong Yang <yangyicong@hisilicon.com> wrote:  
> >> 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:  
> > 
> > ...
> >   
> >>> 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.
> 
Rather than go around a gain, I fixed this up as discussed above and applied
to the togreg branch of iio.git and pushed out as testing.

Thanks,

Jonathan