Hi, not suggesting the diff below, or anything, as i'm not familiar w/that device_lookup() refcounting, nor have time to find out about it right now by opening subr_autoconf.c, but is it even necessary, at all? to unref while erroring out like that? or acceptable, but w/only devices like gpio which don't move around often? :)
-Artturi diff --git a/sys/dev/gpio/gpio.c b/sys/dev/gpio/gpio.c index 0aa7ed5c69b..71b83e44227 100644 --- a/sys/dev/gpio/gpio.c +++ b/sys/dev/gpio/gpio.c @@ -254,8 +254,10 @@ gpioopen(dev_t dev, int flag, int mode, struct proc *p) if (sc == NULL) return (ENXIO); - if (sc->sc_opened) + if (sc->sc_opened) { + device_unref((struct device *)sc); return (EBUSY); + } sc->sc_opened = 1; return (0); @@ -272,6 +274,7 @@ gpioclose(dev_t dev, int flag, int mode, struct proc *p) sc->sc_opened = 0; + device_unref((struct device *)sc); return (0); } @@ -300,6 +303,7 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) struct gpio_pin_set *set; struct device *dv; int pin, value, flags, npins, found; + int rv = 0; sc = (struct gpio_softc *)device_lookup(&gpio_cd, minor(dev)); if (sc == NULL) @@ -324,47 +328,65 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) if (op->gp_name[0] != '\0') { pin = gpio_pinbyname(sc, op->gp_name); - if (pin == -1) - return (EINVAL); + if (pin == -1) { + rv = EINVAL; + goto _fail_unref; + } } else pin = op->gp_pin; - if (pin < 0 || pin >= sc->sc_npins) - return (EINVAL); + if (pin < 0 || pin >= sc->sc_npins) { + rv = EINVAL; + goto _fail_unref; + } if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET) && - securelevel > 0) - return (EPERM); + securelevel > 0) { + rv = EPERM; + goto _fail_unref; + } /* return read value */ op->gp_value = gpiobus_pin_read(gc, pin); break; case GPIOPINWRITE: - if ((flag & FWRITE) == 0) - return (EBADF); + if ((flag & FWRITE) == 0) { + rv = EBADF; + goto _fail_unref; + } op = (struct gpio_pin_op *)data; if (op->gp_name[0] != '\0') { pin = gpio_pinbyname(sc, op->gp_name); - if (pin == -1) - return (EINVAL); + if (pin == -1) { + rv = EINVAL; + goto _fail_unref; + } } else pin = op->gp_pin; - if (pin < 0 || pin >= sc->sc_npins) - return (EINVAL); + if (pin < 0 || pin >= sc->sc_npins) { + rv = EINVAL; + goto _fail_unref; + } - if (sc->sc_pins[pin].pin_mapped) - return (EBUSY); + if (sc->sc_pins[pin].pin_mapped) { + rv = EBUSY; + goto _fail_unref; + } if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET) && - securelevel > 0) - return (EPERM); + securelevel > 0) { + rv = EPERM; + goto _fail_unref; + } value = op->gp_value; - if (value != GPIO_PIN_LOW && value != GPIO_PIN_HIGH) - return (EINVAL); + if (value != GPIO_PIN_LOW && value != GPIO_PIN_HIGH) { + rv = EINVAL; + goto _fail_unref; + } gpiobus_pin_write(gc, pin, value); /* return old value */ @@ -373,27 +395,37 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) sc->sc_pins[pin].pin_state = value; break; case GPIOPINTOGGLE: - if ((flag & FWRITE) == 0) - return (EBADF); + if ((flag & FWRITE) == 0) { + rv = EBADF; + goto _fail_unref; + } op = (struct gpio_pin_op *)data; if (op->gp_name[0] != '\0') { pin = gpio_pinbyname(sc, op->gp_name); - if (pin == -1) - return (EINVAL); + if (pin == -1) { + rv = EINVAL; + goto _fail_unref; + } } else pin = op->gp_pin; - if (pin < 0 || pin >= sc->sc_npins) - return (EINVAL); + if (pin < 0 || pin >= sc->sc_npins) { + rv = EINVAL; + goto _fail_unref; + } - if (sc->sc_pins[pin].pin_mapped) - return (EBUSY); + if (sc->sc_pins[pin].pin_mapped) { + rv = EBUSY; + goto _fail_unref; + } if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET) && - securelevel > 0) - return (EPERM); + securelevel > 0) { + rv = EPERM; + goto _fail_unref; + } value = (sc->sc_pins[pin].pin_state == GPIO_PIN_LOW ? GPIO_PIN_HIGH : GPIO_PIN_LOW); @@ -404,8 +436,10 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) sc->sc_pins[pin].pin_state = value; break; case GPIOATTACH: - if (securelevel > 0) - return (EPERM); + if (securelevel > 0) { + rv = EPERM; + goto _fail_unref; + } attach = (struct gpio_attach *)data; bzero(&ga, sizeof(ga)); @@ -424,8 +458,10 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) } break; case GPIODETACH: - if (securelevel > 0) - return (EPERM); + if (securelevel > 0) { + rv = EPERM; + goto _fail_unref; + } attach = (struct gpio_attach *)data; LIST_FOREACH(gdev, &sc->sc_devs, sc_next) { @@ -440,23 +476,32 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) } break; case GPIOPINSET: - if (securelevel > 0) - return (EPERM); + if (securelevel > 0) { + rv = EPERM; + goto _fail_unref; + } set = (struct gpio_pin_set *)data; if (set->gp_name[0] != '\0') { pin = gpio_pinbyname(sc, set->gp_name); - if (pin == -1) - return (EINVAL); + if (pin == -1) { + rv = EINVAL; + goto _fail_unref; + } } else pin = set->gp_pin; - if (pin < 0 || pin >= sc->sc_npins) - return (EINVAL); + if (pin < 0 || pin >= sc->sc_npins) { + rv = EINVAL; + goto _fail_unref; + } flags = set->gp_flags; /* check that the controller supports all requested flags */ - if ((flags & sc->sc_pins[pin].pin_caps) != flags) - return (ENODEV); + if ((flags & sc->sc_pins[pin].pin_caps) != flags) { + rv = ENODEV; + goto _fail_unref; + } + flags = set->gp_flags | GPIO_PIN_SET; set->gp_caps = sc->sc_pins[pin].pin_caps; @@ -488,24 +533,33 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) } break; case GPIOPINUNSET: - if (securelevel > 0) - return (EPERM); + if (securelevel > 0) { + rv = EPERM; + goto _fail_unref; + } set = (struct gpio_pin_set *)data; if (set->gp_name[0] != '\0') { pin = gpio_pinbyname(sc, set->gp_name); - if (pin == -1) - return (EINVAL); + if (pin == -1) { + rv = EINVAL; + goto _fail_unref; + } } else pin = set->gp_pin; - if (pin < 0 || pin >= sc->sc_npins) - return (EINVAL); - if (sc->sc_pins[pin].pin_mapped) - return (EBUSY); - if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET)) - return (EINVAL); - + if (pin < 0 || pin >= sc->sc_npins){ + rv = EINVAL; + goto _fail_unref; + } + if (sc->sc_pins[pin].pin_mapped) { + rv EBUSY; + goto _fail_unref; + } + if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET)) { + rv = EINVAL; + goto _fail_unref; + } LIST_FOREACH(nm, &sc->sc_names, gp_next) { if (nm->gp_pin == pin) { LIST_REMOVE(nm, gp_next); @@ -516,8 +570,10 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) sc->sc_pins[pin].pin_flags &= ~GPIO_PIN_SET; break; default: - return (ENOTTY); + rv = ENOTTY; + break; } - - return (0); +_fail_unref: + device_unref((struct device *)sc); + return (rv); }