lkml.org 
[lkml]   [2016]   [Nov]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/3] clk: keystone: Fix an error checking
From
Date
Le 02/11/2016 à 01:22, Stephen Boyd a écrit :
> On 10/24, Christophe JAILLET wrote:
>> clk_register_pll() can return ERR_PTR(-ENOMEM) so checking the return value
>> against NULL only is not correct.
> The code just doesn't propagate the error up to the caller.
> Instead the caller treats NULL as an error and non-NULL as valid.
> If the callee detects an error it hides it and returns NULL.

Could you please clarify?
I thought that your point was that 'clk_register_pll()' was returning
NULL when 'clk_register()' was returning an error.
The proposed patch fixes it and now 'clk_register_pll()' returns:
- either ERR_PTR(-ENOMEM) if zkalloc fails
- or clk if 'clk_register()' fails. In this case it is an error pointer
- or a valid, non NULL, pointer in case of success

The caller, '_of_pll_clk_init()' has also been updated to test the
return value using IS_ERR.


So, which caller/callee do you refer to?
Would you like '_of_pll_clk_init()' to also propagate the error?

Or is it the phrasing of the log entry which is not clear enough and
should be updated?

>
>> In order to fix it, update clk_register_pll() to always return an error
>> pointer in case of error and check the return value with IS_ERR.
>>
>> While at it, also fix a tab vs space in the surrounding code.
>>
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> ---
>> Un-compiled and un-tested.
> Please at least compile test patches.
Agreed, and I usually do.
But in this particular case, I don't have the build environment to do it.

I preferred to report what looks like a (small) bug to me and clearly
state that I didn't even compile-tested it rather than just ignoring it.
Hoping that this approach, in such a case, is acceptable.

CJ

\
 
 \ /
  Last update: 2016-11-04 06:44    [W:0.046 / U:1.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site