[PATCH 1/6] clk: Handle error pointers in clk_valid()

[Sean Anderson]

On 2/20/23 05:46, Michal Suchánek wrote:
> On Sun, Feb 19, 2023 at 11:59:34PM -0600, Samuel Holland wrote:
>> Some clk uclass functions, such as devm_clk_get() and clk_get_parent(),
>> return error pointers. clk_valid() should not consider these pointers
>> to be valid.
>>
>> Fixes: 8a1661f20e6c ("drivers: clk: Handle gracefully NULL pointers")
>> Signed-off-by: Samuel Holland <samuel at sholland.org>
>> ---
>>
>>   include/clk.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/clk.h b/include/clk.h
>> index d91285235f7..4acb878ec6d 100644
>> --- a/include/clk.h
>> +++ b/include/clk.h
>> @@ -671,7 +671,7 @@ static inline bool clk_dev_binded(struct clk *clk)
>>    */
>>   static inline bool clk_valid(struct clk *clk)
>>   {
>> -	return clk && !!clk->dev;
>> +	return clk && !IS_ERR(clk) && !!clk->dev;
>>   }
>>   
>>   int soc_clk_dump(void);
> 
> Maybe it would be better to avoid the error pointers instead, there
> should not be that many places where they are returned.

This not quite the right way to phrase this. Instead, I would ask: where
are places where the return value is not checked correctly? It's perfectly
fine to pass NULL or uninitialized clocks to other clock functions, but it's
not OK to ignore errors.

--Sean