[PATCH v6 10/12] watchdog: add gpio watchdog driver

[Simon Glass]

Hi Rasmus,

On Fri, 20 Aug 2021 at 00:22, Rasmus Villemoes
<rasmus.villemoes at prevas.dk> 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