| Message ID | 1437125570-28623-8-git-send-email-mpa@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
On Fri, Jul 17, 2015 at 11:32:48AM +0200, Markus Pargmann wrote: > Signed-off-by: Markus Pargmann <mpa@pengutronix.de> > --- > drivers/gpio/gpiolib-sysfs.c | 41 ++++++++++++++++++++++++++++------------- > 1 file changed, 28 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c > index b57ed8e55ab5..c3b74440ca67 100644 > --- a/drivers/gpio/gpiolib-sysfs.c > +++ b/drivers/gpio/gpiolib-sysfs.c > @@ -443,18 +443,25 @@ static ssize_t export_store(struct class *class, > const char *buf, size_t len) > { > long gpio; > - struct gpio_desc *desc; > + struct gpio_desc *desc = NULL; > int status; > > status = kstrtol(buf, 0, &gpio); > - if (status < 0) > - goto done; > + if (!status) > + desc = gpio_to_desc(gpio); > > - desc = gpio_to_desc(gpio); > - /* reject invalid GPIOs */ > + /* Fall back on detection by name */ > if (!desc) { > - pr_warn("%s: invalid GPIO %ld\n", __func__, gpio); > - return -EINVAL; > + char *gpio_name = kstrdup(buf, GFP_KERNEL); Must check for allocation failures, but why use kstrdup at all? > + > + desc = gpio_name_to_desc(strim(gpio_name)); > + kfree(gpio_name); > + > + /* reject invalid GPIOs */ > + if (!desc) { > + pr_warn("%s: invalid GPIO %s\n", __func__, buf); > + return -EINVAL; > + } > } > > /* No extra locking here; FLAG_SYSFS just signifies that the > @@ -485,17 +492,25 @@ static ssize_t unexport_store(struct class *class, > const char *buf, size_t len) > { > long gpio; > - struct gpio_desc *desc; > + struct gpio_desc *desc = NULL; > int status; > > status = kstrtol(buf, 0, &gpio); > - if (status < 0) > - goto done; > + if (!status) > + desc = gpio_to_desc(gpio); > + > + /* Fall back on detection by name */ > + if (!desc) { > + char *gpio_name = kstrdup(buf, GFP_KERNEL); Same here. > + > + desc = gpio_name_to_desc(strim(gpio_name)); > + kfree(gpio_name); > + } > + Random whitespace change. > > - desc = gpio_to_desc(gpio); > /* reject bogus commands (gpio_unexport ignores them) */ > if (!desc) { > - pr_warn("%s: invalid GPIO %ld\n", __func__, gpio); > + pr_warn("%s: invalid GPIO %s\n", __func__, buf); > return -EINVAL; > } Johan -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 28, 2015 at 11:50:04AM +0200, Johan Hovold wrote: > On Fri, Jul 17, 2015 at 11:32:48AM +0200, Markus Pargmann wrote: > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de> > > --- > > drivers/gpio/gpiolib-sysfs.c | 41 ++++++++++++++++++++++++++++------------- > > 1 file changed, 28 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c > > index b57ed8e55ab5..c3b74440ca67 100644 > > --- a/drivers/gpio/gpiolib-sysfs.c > > +++ b/drivers/gpio/gpiolib-sysfs.c > > @@ -443,18 +443,25 @@ static ssize_t export_store(struct class *class, > > const char *buf, size_t len) > > { > > long gpio; > > - struct gpio_desc *desc; > > + struct gpio_desc *desc = NULL; > > int status; > > > > status = kstrtol(buf, 0, &gpio); > > - if (status < 0) > > - goto done; > > + if (!status) > > + desc = gpio_to_desc(gpio); > > > > - desc = gpio_to_desc(gpio); > > - /* reject invalid GPIOs */ > > + /* Fall back on detection by name */ > > if (!desc) { > > - pr_warn("%s: invalid GPIO %ld\n", __func__, gpio); > > - return -EINVAL; > > + char *gpio_name = kstrdup(buf, GFP_KERNEL); > > Must check for allocation failures, but why use kstrdup at all? Yes have to check that. strim() may modify the buf variable which is constant. Best regards, Markus > > > + > > + desc = gpio_name_to_desc(strim(gpio_name)); > > + kfree(gpio_name); > > + > > + /* reject invalid GPIOs */ > > + if (!desc) { > > + pr_warn("%s: invalid GPIO %s\n", __func__, buf); > > + return -EINVAL; > > + } > > } > > > > /* No extra locking here; FLAG_SYSFS just signifies that the > > @@ -485,17 +492,25 @@ static ssize_t unexport_store(struct class *class, > > const char *buf, size_t len) > > { > > long gpio; > > - struct gpio_desc *desc; > > + struct gpio_desc *desc = NULL; > > int status; > > > > status = kstrtol(buf, 0, &gpio); > > - if (status < 0) > > - goto done; > > + if (!status) > > + desc = gpio_to_desc(gpio); > > + > > + /* Fall back on detection by name */ > > + if (!desc) { > > + char *gpio_name = kstrdup(buf, GFP_KERNEL); > > Same here. > > > + > > + desc = gpio_name_to_desc(strim(gpio_name)); > > + kfree(gpio_name); > > + } > > + > > Random whitespace change. > > > > > - desc = gpio_to_desc(gpio); > > /* reject bogus commands (gpio_unexport ignores them) */ > > if (!desc) { > > - pr_warn("%s: invalid GPIO %ld\n", __func__, gpio); > > + pr_warn("%s: invalid GPIO %s\n", __func__, buf); > > return -EINVAL; > > } > > Johan >
On Wed, Jul 29, 2015 at 08:57:36AM +0200, Markus Pargmann wrote: > On Tue, Jul 28, 2015 at 11:50:04AM +0200, Johan Hovold wrote: > > On Fri, Jul 17, 2015 at 11:32:48AM +0200, Markus Pargmann wrote: > > > + char *gpio_name = kstrdup(buf, GFP_KERNEL); > > > > Must check for allocation failures, but why use kstrdup at all? > > Yes have to check that. > strim() may modify the buf variable which is constant. > > > > + > > > + desc = gpio_name_to_desc(strim(gpio_name)); Ah, I missed that strim. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c index b57ed8e55ab5..c3b74440ca67 100644 --- a/drivers/gpio/gpiolib-sysfs.c +++ b/drivers/gpio/gpiolib-sysfs.c @@ -443,18 +443,25 @@ static ssize_t export_store(struct class *class, const char *buf, size_t len) { long gpio; - struct gpio_desc *desc; + struct gpio_desc *desc = NULL; int status; status = kstrtol(buf, 0, &gpio); - if (status < 0) - goto done; + if (!status) + desc = gpio_to_desc(gpio); - desc = gpio_to_desc(gpio); - /* reject invalid GPIOs */ + /* Fall back on detection by name */ if (!desc) { - pr_warn("%s: invalid GPIO %ld\n", __func__, gpio); - return -EINVAL; + char *gpio_name = kstrdup(buf, GFP_KERNEL); + + desc = gpio_name_to_desc(strim(gpio_name)); + kfree(gpio_name); + + /* reject invalid GPIOs */ + if (!desc) { + pr_warn("%s: invalid GPIO %s\n", __func__, buf); + return -EINVAL; + } } /* No extra locking here; FLAG_SYSFS just signifies that the @@ -485,17 +492,25 @@ static ssize_t unexport_store(struct class *class, const char *buf, size_t len) { long gpio; - struct gpio_desc *desc; + struct gpio_desc *desc = NULL; int status; status = kstrtol(buf, 0, &gpio); - if (status < 0) - goto done; + if (!status) + desc = gpio_to_desc(gpio); + + /* Fall back on detection by name */ + if (!desc) { + char *gpio_name = kstrdup(buf, GFP_KERNEL); + + desc = gpio_name_to_desc(strim(gpio_name)); + kfree(gpio_name); + } + - desc = gpio_to_desc(gpio); /* reject bogus commands (gpio_unexport ignores them) */ if (!desc) { - pr_warn("%s: invalid GPIO %ld\n", __func__, gpio); + pr_warn("%s: invalid GPIO %s\n", __func__, buf); return -EINVAL; } @@ -509,7 +524,7 @@ static ssize_t unexport_store(struct class *class, status = 0; gpiod_free(desc); } -done: + if (status) pr_debug("%s: status %d\n", __func__, status); return status ? : len;
Signed-off-by: Markus Pargmann <mpa@pengutronix.de> --- drivers/gpio/gpiolib-sysfs.c | 41 ++++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-)