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]

Date:	Wed, 2 Jan 2013 10:15:54 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Julia Lawall <julia.lawall@...6.fr>
Cc:	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 Wed, Jan 02, 2013 at 10:44:32AM +0100, Julia Lawall wrote:
> On Wed, 2 Jan 2013, Russell King - ARM Linux wrote:
> 
> > On Wed, Jan 02, 2013 at 08:10:36AM +0300, Dan Carpenter wrote:
> > > clk_get() returns NULL if CONFIG_HAVE_CLK is disabled.
> > >
> > > I told Tony about this but everyone has been gone with end of year
> > > holidays so it hasn't been addressed.
> > >
> > > Tony, please fix it so people don't apply these patches until
> > > clk_get() is updated to not return NULL.  It sucks to have to revert
> > > patches.
> >
> > How about people stop using IS_ERR_OR_NULL for stuff which it shouldn't
> > be used for?
> 
> Perhaps the cases where clk_get returns NULL could have a comment
> indicating that NULL does not represent a failure?

No.  More documentation is never the answer to people not reading
existing documentation.

> In 3.7.1, it looks like it might have been possible for NULL to be
> returned by clk_get in arch/mips/loongson1/common/clock.c, but that
> definition seems to be gone in a recent linux-next.  The remaining
> definitions look OK.

How about people just read the API and comply with it rather than
doing their own thing all the time?  We've already had at least one
instance where someone has tried using IS_ERR() with the ioremap()
return value.

Really, if you're going to program kernel space, it is very important
that you *know* the interfaces that you're using and you comply with
them.  Otherwise, you have no business saying that you're a kernel
programmer.

Yes, the odd mistake happens, but that's no excuse for the constant
blatent mistakes with stuff like IS_ERR_OR_NULL() with clk_get()
which just comes from total laziness on the part of the coder to
understand the interfaces being used.  Hell, it's even documented
in linux/clk.h - that just shows how many people read the
documentation which has been around since the clk API came about.
--
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/