| 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
| ||
|
Date: Mon, 22 Oct 2012 11:39:12 +1100
From: Ryan Mallon <rmallon@...il.com>
To: Linus Walleij <linus.walleij@...aro.org>,
Grant Likely <grant.likely@...retlab.ca>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: [PATCH v2] gpiolib: Refactor gpio_export
The gpio_export function uses nested if statements and the status
variable to handle the failure cases. This makes the function logic
difficult to follow. Refactor the code to abort immediately on failure
using goto. This makes the code slightly longer, but significantly
reduces the nesting and number of split lines and makes the code easier
to read.
Signed-off-by: Ryan Mallon
---
Changes since v1:
- Return immediately if no unwinding needed
- Added pr_debug to immediate returns
- Changed unavailable (!requested || exported) to -EPERM
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 5d6c71e..d5f9742 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -702,8 +702,9 @@ int gpio_export(unsigned gpio, bool direction_may_change)
{
unsigned long flags;
struct gpio_desc *desc;
- int status = -EINVAL;
+ int status;
const char *ioname = NULL;
+ struct device *dev;
/* can't export until sysfs is available ... */
if (!gpio_class.p) {
@@ -711,59 +712,65 @@ int gpio_export(unsigned gpio, bool direction_may_change)
return -ENOENT;
}
- if (!gpio_is_valid(gpio))
- goto done;
+ if (!gpio_is_valid(gpio)) {
+ pr_debug("%s: gpio %d is not valid\n", __func__, gpio);
+ return -EINVAL;
+ }
mutex_lock(&sysfs_lock);
spin_lock_irqsave(&gpio_lock, flags);
desc = &gpio_desc[gpio];
- if (test_bit(FLAG_REQUESTED, &desc->flags)
- && !test_bit(FLAG_EXPORT, &desc->flags)) {
- status = 0;
- if (!desc->chip->direction_input
- || !desc->chip->direction_output)
- direction_may_change = false;
+ if (!test_bit(FLAG_REQUESTED, &desc->flags) ||
+ test_bit(FLAG_EXPORT, &desc->flags)) {
+ spin_unlock_irqrestore(&gpio_lock, flags);
+ pr_debug("%s: gpio %d unavailable (requested=%d, exported=%d)\n",
+ __func__, gpio,
+ test_bit(FLAG_REQUESTED, &desc->flags),
+ test_bit(FLAG_EXPORT, &desc->flags));
+ return -EPERM;
}
+
+ if (!desc->chip->direction_input || !desc->chip->direction_output)
+ direction_may_change = false;
spin_unlock_irqrestore(&gpio_lock, flags);
if (desc->chip->names && desc->chip->names[gpio - desc->chip->base])
ioname = desc->chip->names[gpio - desc->chip->base];
- if (status == 0) {
- struct device *dev;
-
- dev = device_create(&gpio_class, desc->chip->dev, MKDEV(0, 0),
- desc, ioname ? ioname : "gpio%u", gpio);
- if (!IS_ERR(dev)) {
- status = sysfs_create_group(&dev->kobj,
- &gpio_attr_group);
-
- if (!status && direction_may_change)
- status = device_create_file(dev,
- &dev_attr_direction);
-
- if (!status && gpio_to_irq(gpio) >= 0
- && (direction_may_change
- || !test_bit(FLAG_IS_OUT,
- &desc->flags)))
- status = device_create_file(dev,
- &dev_attr_edge);
-
- if (status != 0)
- device_unregister(dev);
- } else
- status = PTR_ERR(dev);
- if (status == 0)
- set_bit(FLAG_EXPORT, &desc->flags);
+ dev = device_create(&gpio_class, desc->chip->dev, MKDEV(0, 0),
+ desc, ioname ? ioname : "gpio%u", gpio);
+ if (IS_ERR(dev)) {
+ status = PTR_ERR(dev);
+ goto fail_unlock;
}
- mutex_unlock(&sysfs_lock);
-
-done:
+ status = sysfs_create_group(&dev->kobj, &gpio_attr_group);
if (status)
- pr_debug("%s: gpio%d status %d\n", __func__, gpio, status);
+ goto fail_unregister_device;
+
+ if (direction_may_change) {
+ status = device_create_file(dev, &dev_attr_direction);
+ if (status)
+ goto fail_unregister_device;
+ }
+ if (gpio_to_irq(gpio) >= 0 && (direction_may_change ||
+ !test_bit(FLAG_IS_OUT, &desc->flags))) {
+ status = device_create_file(dev, &dev_attr_edge);
+ if (status)
+ goto fail_unregister_device;
+ }
+
+ set_bit(FLAG_EXPORT, &desc->flags);
+ mutex_unlock(&sysfs_lock);
+ return 0;
+
+fail_unregister_device:
+ device_unregister(dev);
+fail_unlock:
+ mutex_unlock(&sysfs_lock);
+ pr_debug("%s: gpio%d status %d\n", __func__, gpio, status);
return status;
}
EXPORT_SYMBOL_GPL(gpio_export);
--
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/
Powered by blists - more mailing lists