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

[Rolf Eike Beer <eike-kernel@sf-tec.de>]

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.

This is unneeded here as it's embedded into struct rtc_device. Jonathan?

>  other drivers have implemented different error paths.
>  which one is correct?

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.

Eike

Attachment: pgp7s18MkpMPT.pgp
Description: PGP signature