linux-kernel - Re: [RTC] Remove superfluous call to call to cdev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 21 Sep 2006 10:30:02 +0200
From:	Rolf Eike Beer <eike-kernel@...tec.de>
To:	Alessandro Zummo <alessandro.zummo@...ertech.it>
Cc:	Andrew Morton <akpm@...l.org>, linux-kernel@...r.kernel.org,
	Jonathan Corbet <corbet@....net>
Subject: Re: [RTC] Remove superfluous call to call to cdev_del()

Alessandro Zummo wrote:
> On Thu, 21 Sep 2006 09:46:06 +0200
>
> Rolf Eike Beer <eike-kernel@...tec.de> wrote:
> > If cdev_add() fails there is no good reason to call cdev_del().
> >
> > Signed-off-by: Rolf Eike Beer <eike-kernel@...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

Content of type "application/pgp-signature" skipped