Re: [PATCH 2/4] arch/arm/mach-at91/clock.c: Add missing IS_ERR test

[Julia Lawall]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/