Messages in this thread | | | Date | Thu, 21 Sep 2006 10:44:24 -0400 | From | "Dmitry Torokhov" <> | Subject | Re: [RTC] Remove superfluous call to call to cdev_del() |
| |
On 9/21/06, Rolf Eike Beer <eike-kernel@sf-tec.de> wrote: > Alessandro Zummo wrote: > > On Thu, 21 Sep 2006 09:46:06 +0200 > > > > Rolf Eike Beer <eike-kernel@sf-tec.de> wrote: > > > If cdev_add() fails there is no good reason to call cdev_del(). > > > > > > Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de> > > > > > > rtc->char_dev.owner = rtc->owner; > > > > > > if (cdev_add(&rtc->char_dev, MKDEV(MAJOR(rtc_devt), rtc->id), 1)) { > > > - cdev_del(&rtc->char_dev); > > > dev_err(class_dev->dev, > > > "failed to add char device %d:%d\n", > > > MAJOR(rtc_devt), rtc->id); > > > > I'm not sure.. this is drivers/char/raw.c: > > > > > > cdev_init(&raw_cdev, &raw_fops); > > if (cdev_add(&raw_cdev, dev, MAX_RAW_MINORS)) { > > kobject_put(&raw_cdev.kobj); > > unregister_chrdev_region(dev, MAX_RAW_MINORS); > > goto error; > > } > > > > in case of failure, it does a kobject_put. > > tha same call is done by cdev_del.
But cdev_del also tries to do kobj_unmap before doing kobject_put. If cdev_add fails the object is not added to the map so we shoult not try to unmap it (although it does not hurt in the current implementation).
> > This is unneeded here as it's embedded into struct rtc_device. Jonathan? >
cdev distingueshes between stattically and synamically allocated objects and so it is safe to do kobject_put/cdev_del even on cdevs embedded into other structures.
> > other drivers have implemented different error paths. > > which one is correct? >
raw.c seems to be DTRT.
> Probably half of them are wrong, ugly or both. I think this interface is not > very intuitive at all. This two calls needed to set up an embedded cdev are > IMHO the best way to keep people confused. At least the (possible) need to > call cdev_del() on failed cdev_add() is just weird. >
The rule is simple - after cdev_init/cdev_alloc call kobject_put. After successful cdev_add call cdev_del.
-- Dmitry - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |