Re: [PATCH] gpiolib: check if gpio_desc pointer is error or NULL

From: Ben Dooks
Date: Wed Mar 19 2014 - 04:33:47 EST

On 19/03/14 02:48, Alexandre Courbot wrote:
On Tue, Mar 18, 2014 at 7:41 PM, Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx> wrote:
Some of the gpiod_ calls take a pointer to a gpio_desc as their
argument but only check to see if it is NULL to validate the

Calls such as devm_gpiod_get() return an error-pointer if they
fail, so doing the following will not work:

gpio = devm_gpiod_get(...);
gpiod_direction_output(gpio, 0);

The sequence produces an OOPS like:

Unable to handle kernel paging request at virtual address fffffffe

Change all calls that check for !desc to use IS_ERR_OR_NULL() to
avoid these issues.

This change is certainly correct from a semantics point of view. Maybe
I'd argue that the burden is on the caller to check that gpiod_get()
returns a valid GPIO descriptor, but having extra security doesn't
hurt. However my problem with this change in its current form is that
it will hide such forgetfulnesses by making functions like
gpiod_get_value() fail silently instead of triggering a oops.

On the other hand, it means that we do not have to keep checking
the validity of the pointer in the caller.

Could you make sure that any call of a gpiolib function on a non-valid
descriptor raises at least a message in the console, and while you are
at it harmonize the way these messages are output? Right now we are
using pr_debug(), pr_warn() or WARN_ON() depending on the location. I
would say that using an invalid GPIO descriptor is offending enough
that it should trigger a stack dump at every attempt.

Ben Dooks
Senior Engineer Codethink - Providing Genius
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at