On Fri, 9 Apr 2021 21:05:49 +0800
Yicong Yang <yangyicong@xxxxxxxxxxxxx> wrote:
> 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.
>
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
