[PATCH 01/22] clk: check hw and hw->dev before dereference it

[Simon Glass]

Hi Claudiu,

On Tue, 4 Aug 2020 at 09:25, <Claudiu.Beznea at microchip.com> wrote:
>
> Hi Simon,
>
> On 04.08.2020 18:08, Simon Glass wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Hi Claudiu,
> >
> > On Tue, 4 Aug 2020 at 01:19, <Claudiu.Beznea at microchip.com> wrote:
> >>
> >>
> >>
> >> On 04.08.2020 05:00, Simon Glass wrote:
> >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >>>
> >>> Hi Claudiu,
> >>>
> >>> On Wed, 29 Jul 2020 at 08:51, Claudiu Beznea
> >>> <claudiu.beznea at microchip.com> wrote:
> >>>>
> >>>> Check hw and hw->dev before dereference it.
> >>>>
> >>>> Signed-off-by: Claudiu Beznea <claudiu.beznea at microchip.com>
> >>>> ---
> >>>>  drivers/clk/clk.c | 3 +++
> >>>>  1 file changed, 3 insertions(+)
> >>>>
> >>>
> >>> Why is this needed? It adds to code size and these situations should
> >>> not occur. Perhaps use assert()?
> >>
> >> In my debugging, investigating the issues that patches 03/22, 04/22, 06/22
> >> try to address, I reached also this function and checked these pointers. In
> >> the end the issue was not related to them but I though it might be useful
> >> to keep these in a patch. I will remove it in the next version.
> >
> > IMO we should use assert() to check invariants and catch basic
> > programming errors. But production testing should make sure that the
> > software basically works.
> >
> > Of course it is nice to have these checks, but they add to code size
> > which is always a concern. So I think we should rely on assert() to
> > catch the errors during development, so we are not wasting code
> > checking for things that we know cannot happen.
>
> OK, I'll switch to assert().

One more point I should have made is that my comments apply mostly to
common code that everyone has to use - e.g. the core clock code. So if
you want to put dev_err() and other things in your driver and you know
about the code-size implications that is less of a concern. But with
common code, we should be careful.

Regards,
Simon