[PATCH RESEND 6/6] clk: s5p-g2d: Fix incorrect usage of IS_ERR_OR_NULL

[linux@arm.linux.org.uk (Russell King - ARM Linux)]

On Thu, Jan 03, 2013 at 02:10:40PM +0300, Dan Carpenter wrote:
> Come on...  Don't say we haven't read comment.  Obviously, the first
> thing we did was read that comment.  I've read it many times at this
> point and I still think we should add in a bit which says:

So where does it give you in that comment permission to treat NULL any
differently to any other non-IS_ERR() return value?

It is very clear: values where IS_ERR() is true are considered errors.
Everything else is considered valid.

> "NOTE:  Drivers should treat the return value as an opaque cookie
> and not dereference it.  NULL returns don't imply an error so don't
> use IS_ERR_OR_NULL() to check for errors."

No.  The one thing I've learnt through maintaining www.arm.linux.org.uk
is that the more of these kinds of "lets add to documentation" suggestions
you get, the more _unclear_ the documentation becomes, and the more it is
open to bad interpretation, and the more suggestions to add more words you
receive.

Concise documentation is the only way to go.  And what we have there today
is concise and to the point.  It specifies it very clearly:

 * Returns a struct clk corresponding to the clock producer, or
 * valid IS_ERR() condition containing errno.

That one sentence gives you all the information you need about it's return
value.  It gives you two choices.  (1) a return value where IS_ERR() is
true, which is an error, and (2) a return value where IS_ERR() is false,
which is a valid cookie.

Maybe you don't realise, but IS_ERR(NULL) is false.  Therefore, this falls
into category (2).

You can't get clearer than that, unless you don't understand the IS_ERR()
and associated macro.

Moreover, it tells you the function to use to check the return value for
errors.  IS_ERR().  It doesn't say IS_ERR_OR_NULL(), it says IS_ERR().

All it takes is for people to engage their grey cells and read the
documentation as it stands, rather than trying to weasel their way around
it and invent crap that it doesn't say.