Re: [PATCH 1/3] clk: keystone: Fix an error checking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux