On 9/21/06, Rolf Eike Beer <[email protected]> wrote:
Alessandro Zummo wrote:
> On Thu, 21 Sep 2006 09:46:06 +0200
>
> Rolf Eike Beer <[email protected]> wrote:
> > If cdev_add() fails there is no good reason to call cdev_del().
> >
> > Signed-off-by: Rolf Eike Beer <[email protected]>
> >
> > 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
