linux-kernel - Re: [PATCH 2/4] arch/arm/mach-at91/clock.c: Add missing IS

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:	Tue, 25 Jan 2011 11:26:41 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Julia Lawall <julia@...u.dk>
Cc:	walter harms <wharms@....de>, Vasiliy Kulikov <segooon@...il.com>,
	Ryan Mallon <ryan@...ewatersys.com>,
	kernel-janitors@...r.kernel.org,
	Nicolas Ferre <nicolas.ferre@...el.com>,
	Jean-Christophe PLAGNIOL-VILLARD <plagnioj@...osoft.com>,
	Andrew Victor <linux@...im.org.za>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] arch/arm/mach-at91/clock.c: Add missing IS_ERR test

On Tue, Jan 25, 2011 at 12:18:40PM +0100, Julia Lawall wrote:
> On Tue, 25 Jan 2011, walter harms wrote:
> > So these is a bug ? They should return -ENOENT ?
> > 
> > The interessting question is: what to do with an error ?
> > 
> > Obviously some architecture can live with NULL, so it is not an critical
> > error. An the patch shows a code that is simply a return, not even the
> > user is informed that something did not work as expected.
> > 
> > From that point of view i would like question if it is useful to have
> > a "detailed" error instead of just returning NULL.
> 
> Somewhat unrelatedly, I often run into code where error handling code is 
> needed, but not present, and the function returns void, so nothing is 
> provided for propagating the error further.  I generally consider these 
> cases to be beyond my expertise to fix...

That is a pain, but so is returning NULL in error conditions.  If you've
got several layers of nesting, and every level returns NULL on error,
it's an awful lot of debugging to find out _why_ a failure happened.

With error codes, it narrows down the number of places which could have
returned that error code, and as error codes can be descriptive, it
turns it into an "oh, I forgot about doing X" or "it's failing *there*"
rather than a puzzle.

The only place where it really makes sense to return NULL is with memory
allocators.  NULL is an accepted value for meaning "I couldn't allocate
memory" as its not a useful pointer value.

The alternative is to have an API like:

	struct clk *clk_get(int *error, ...)
or
	int clk_get(struct clk **, ...)

but that then leads to _additional_ errors made by driver authors and by
implementations - you can no longer guarantee that *error will always be
initialized, and this is why the whole ERR_PTR/PTR_ERR/IS_ERR stuff was
implemented.  The kernel used to have such things in it and they were
buggy.
--
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/