Re: [PATCH 1/3] clk: keystone: Fix an error checking
From: Christophe JAILLET
Date: Fri Nov 04 2016 - 01:44:24 EST
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
The code just doesn't propagate the error up to the caller.
against NULL only is not correct.
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?
Agreed, and I usually do.
In order to fix it, update clk_register_pll() to always return an error
Please at least compile test patches.
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@xxxxxxxxxx>
Un-compiled and un-tested.
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.