Hi Rasmus, On Fri, 20 Aug 2021 at 00:22, Rasmus Villemoes <[email protected]> wrote: > > On 19/08/2021 14.32, Wolfgang Denk wrote: > > > The existence of bad code is not a justification to add more of it. > > Obviously true and I agree. > > However, it is at the same time completely irrelevant in this context, > because the pattern of using the return value of dev_get_priv() without > a NULL check is neither bad or wrong, as has now been explained to you > several times. > > If you really think checking the return value of dev_get_priv() must be > done religiously, perhaps you could tap Stefan (737c3de09984), Marek > (7e1f1e16fe75), or Heiko (6e31c62a175c) on the shoulder and tell them to > stop cranking out "bad" code. > > On 19/08/2021 16.16, Wolfgang Denk wrote: > > > I mean, look at the implementation of dev_get_priv(): > > > > 628 void *dev_get_priv(const struct udevice *dev) > > 629 { > > 630 if (!dev) { > > 631 dm_warn("%s: null device\n", __func__); > > 632 return NULL; > > 633 } > > 634 > > 635 return dm_priv_to_rw(dev->priv_); > > 636 } > > > > If there is guaranteed no way that dev_get_priv() can return a NULL > > pointer, that means that it must be guaranteed that the "dev" > > argument can never be a NULL pointer, either. > > There's another logical fallacy right here. Sure, you've found an input > value for which dev_get_priv() would return NULL. But any caller who > knows they're not passing a NULL dev also know they won't follow that > code path. > > A driver which doesn't populate the priv field by via a non-zero > .priv_auto field may need to check the return value of dev_get_priv(). > I'm not claiming that checking that is always redundant. However, > neither is it anywhere near true that checking is always required.
Just on this point, drivers should use the priv pointer without priv_auto. If we do introduce run-time checks, it would fail with those. Regards, Simon
