Messages in this thread | | | Date | Mon, 11 Feb 2013 15:07:09 +0000 | From | Russell King - ARM Linux <> | Subject | Re: [PATCH 08/33] gpio: Convert to devm_ioremap_resource() |
| |
On Mon, Feb 11, 2013 at 02:53:47PM +0100, Linus Walleij wrote: > NB: among the error codes people want to propagate from > consumer interfaces such as say, clk_get(), regulator_get() > and pinctrl_get() is -EPROBE_DEFER. So just "something > failed" (return NULL) isn't enough. > > We then obviously need to return an int as error code instead > and pass the pointer as argument, so do you mean we should > refactor all the *_get() things from e.g.: > > struct clk *clk_get(struct device *dev, const char *id); > > into something like: > > int clk_get(struct clk **clk, struct device *dev, const char *id); > > across the entire kernel?
I really don't want to do that. What we have today I think is far better than having to munge things like that - you just get a different set of problems through doing that, such as what happens if people ignore the returned error code, etc.
We _should_ really be able to get Sparse to deal with this. Sparse really should be taught about a pointer attribute that says "this is fine to dereference, but it should also have the magic IS_ERR() check done on it too." As IS_ERR() is a function, we can surely mark the passed pointer in some way to tell sparse "this has been appropraitely checked".
Practically, we don't end up with that many problems though with people failing to check the returned pointer (I've seen relatively few of these in the bigger scheme of things). What I have seen through is repeated confusion between IS_ERR() and IS_ERR_OR_NULL() since the second one was introduced - which then leads people into coding their error paths such that they return zero should NULL be returned.
I've said before - almost every usage of IS_ERR_OR_NULL() appears to be a bug... and I've been mooting its removal. I have acks against a patch which deprecates it - but we can't deprecate it until we've removed most of the existing users of it (otherwise akpm will hunt down those who added the deprecated marker.)
| |