Re: [RTC] Remove superfluous call to call to cdev_del()

["Dmitry Torokhov" <dmitry.torokhov@gmail.com>]

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/