30.09.2020 09:13, Nicolin Chen пишет: > On Wed, Sep 30, 2020 at 09:10:38AM +0300, Dmitry Osipenko wrote: >> 30.09.2020 08:49, Nicolin Chen пишет: >>> On Wed, Sep 30, 2020 at 08:11:52AM +0300, Dmitry Osipenko wrote: >>>> 30.09.2020 03:30, Nicolin Chen пишет: >>>>> + /* An invalid mc pointer means mc and smmu drivers are not ready */ >>>>> + if (IS_ERR_OR_NULL(mc)) >>>> >>>> tegra_get_memory_controller() doesn't return NULL. >>> >>> Well, I don't want to assume that it'd do that forever, and the >>> NULL check of IS_ERR_OR_NULL is marked "unlikely" so it doesn't >>> hurt to have. >>> >> >> I don't see any reasons why it won't do that forever. >> >> Secondly, public function can't be changed randomly without updating all >> the callers. >> >> Hence there is no need to handle cases that can't ever happen and it >> hurts readability of the code + original error code is missed. > > I don't quite understand why an extra "_OR_NULL" would hurt > readability....but I'd take a step back and use IS_ERR(). > The tegra_get_memory_controller() doesn't return NULL, hence the NULL-check is misleading. If I was reading that code for the first time and notice such a thing, then instantly I'd have a much lower credibility to the whole code.
