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]

Message-ID: <Pine.LNX.4.64.1101251231060.12123@pc-004.diku.dk>
Date:	Tue, 25 Jan 2011 12:31:55 +0100 (CET)
From:	Julia Lawall <julia@...u.dk>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
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, 25 Jan 2011, Russell King - ARM Linux wrote:

> 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.

I agree that error codes are very useful.  The problem is rather how to 
propagate any sort of error indicator, whether ERR_PTR, NULL, an negative 
integer, etc.

julia
--
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/