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.