Re: [PATCH 6/9] gpiolib: use descriptors internally

[Paul Mundt]

On Mon, Feb 11, 2013 at 03:09:21PM +0100, Linus Walleij wrote:
> On Sat, Feb 9, 2013 at 10:17 AM, Grant Likely <grant.likely@xxxxxxxxxxxx> wrote:
> 
> > The ERR_PTR()/IS_ERR() is a horrible pattern for code
> > readability because it breaks the expectations that programmers have for
> > what is and is not a bad pointer. There are decades of history where the
> > test for a bad pointer is 'if (!ptr)'. Not only does ERR_PTR make make
> > that test not work, but the compiler won't tell you when you get it
> > wrong.
> >
> > There are places where ERR_PTR makes sense. Particularly when
> > communicating with userspace where error codes have very specific
> > meanings, but I don't want it in the GPIO subsystem.
> 
> OK I disagree but you get to decide.
> 
> However if you take this all the way to the descriptor API
> it will make the consumer (driver) API for GPIO descriptors deviate
> from what is today used for clocks, regulators and pins.
> 
> With all the resulting confusion for users.
> I've seen worse subsystem deviations though.
> 
I've always considered it to be more of a complexity issue. If an
interface can fail for half a dozen different reasons, it's useful to be
able to encapsulate this and pass it down the line to the consumer
(particularly in cases where no useful debug output is provided, and the
first thing someone is going to do is go and instrument the registration
path with printks to figure out where things went wrong). In the case
where the work to do is relatively straightforward and there's not much
complexity, NULL is clearly the way to go.

There are already quite a few cases today converting NULLs over to
arbitrary ERR_PTR values or IS_ERR cases wrapping back to NULL, so it's
clear that both have to be supported interacting with one another
regardless. Then there's that whole IS_ERR_OR_NULL case which everyone
seems to get wrong, but that's another issue entirely.
--
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/