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] [day] [month] [year] [list]

Message-ID: <d120d5000609210744w4c52bcf0qda2c3b9f12d4c2d3@mail.gmail.com>
Date:	Thu, 21 Sep 2006 10:44:24 -0400
From:	"Dmitry Torokhov" <dmitry.torokhov@...il.com>
To:	"Rolf Eike Beer" <eike-kernel@...tec.de>
Cc:	"Alessandro Zummo" <alessandro.zummo@...ertech.it>,
	"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()

On 9/21/06, Rolf Eike Beer <eike-kernel@...tec.de> wrote:
> 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.

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/