linux-kernel - Re: [PATCH RESEND 6/6] clk: s5p-g2d: Fix incorrect usage of IS_ERR_OR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130103112102.GM2631@n2100.arm.linux.org.uk>
Date:	Thu, 3 Jan 2013 11:21:02 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Dan Carpenter <dan.carpenter@...cle.com>
Cc:	Tony Prisk <linux@...sktech.co.nz>,
	Tomasz Stanislawski <t.stanislaws@...sung.com>,
	Dan Carpenter <error27@...il.com>,
	Sergei Shtylyov <sshtylyov@...sta.com>,
	kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org,
	Kyungmin Park <kyungmin.park@...sung.com>,
	Sylwester Nawrocki <sylvester.nawrocki@...il.com>,
	linux-arm-kernel@...ts.infradead.org, linux-media@...r.kernel.org
Subject: Re: [PATCH RESEND 6/6] clk: s5p-g2d: Fix incorrect usage of
	IS_ERR_OR_NULL

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/