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

[Claudiu.Beznea at microchip.com]

On 04.08.2020 18:29, 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 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.

Sure!

Thank you,
Claudiu Beznea

> 
> Regards,
> Simon
>