From: Greg KH <greg@kroah.com>
To: Boaz Harrosh <bharrosh@panasas.com>
Cc: Kay Sievers <kay.sievers@vrfy.org>,
Andrew Morton <akpm@linux-foundation.org>,
Vasiliy Kulikov <segooon@gmail.com>,
kernel-janitors@vger.kernel.org, Tejun Heo <tj@kernel.org>,
Jiri Slaby <jirislaby@gmail.com>,
linux-kernel@vger.kernel.org,
James Bottomley <James.Bottomley@suse.de>,
Dan Carpenter <error27@gmail.com>
Subject: Re: [PATCH 04/14] memstick: core: fix device_register() error handling
Date: Wed, 22 Sep 2010 08:47:46 -0700 [thread overview]
Message-ID: <20100922154746.GB26178@kroah.com> (raw)
In-Reply-To: <4C99D436.700@panasas.com>
On Wed, Sep 22, 2010 at 12:02:30PM +0200, Boaz Harrosh wrote:
> On 09/22/2010 10:53 AM, Kay Sievers wrote:
> > On Wed, Sep 22, 2010 at 00:49, Greg KH <greg@kroah.com> wrote:
> >
> >> int device_register(struct device *dev)
> >> {
> >> + int retval;
> >> +
> >> device_initialize(dev);
> >> - return device_add(dev);
> >> + retval = device_add(dev);
> >> + if (retval)
> >> + put_device(dev);
> >> + return retval;
> >> }
> >
> >> Kay, what am I missing here, why can't we just do this? Hm, the
> >> side-affect might be that if device_register() fails, NO ONE had better
> >> touch that device again, as it might have just been freed from the
> >> system. I wonder if that will cause problems...
> >
> > That looks right, besides that there might be callers already doing
> > this. Which needs to be checked.
> >
> > I never liked this pretty useless "convenience API", which just wraps
> > two simple functions and the first one can never fail anyway.
> >
> > We better remove that device_register() stuff entirely in the long
> > run, it's not doing any good. At the kobject level we killed the same
> > stuff already long ago.
> >
>
> That would be fine, and ping me when you do it, I'll help with my
> driver. But don't forget to let us have a way to embed a device inside
> a bigger structure.
That's what you should be doing anyway, so it shouldn't be a problem.
> For meanwhile Please check the patch James sent to add_device that cleans
> up the allocation of the kobj.name member. (And the comment made there)
See my other email why that isn't a good idea.
thanks,
greg k-h
next prev parent reply other threads:[~2010-09-22 15:50 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-19 12:54 [PATCH 04/14] memstick: core: fix device_register() error handling Vasiliy Kulikov
2010-09-21 22:20 ` Andrew Morton
2010-09-21 22:49 ` Greg KH
2010-09-22 8:53 ` Kay Sievers
2010-09-22 10:02 ` Boaz Harrosh
2010-09-22 15:47 ` Greg KH [this message]
2010-09-22 15:56 ` James Bottomley
2010-09-22 16:20 ` Greg KH
2010-09-22 16:23 ` James Bottomley
2010-09-22 15:50 ` Greg KH
2010-09-23 12:10 ` Vasiliy Kulikov
2010-09-22 9:58 ` Boaz Harrosh
2010-09-22 15:46 ` Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100922154746.GB26178@kroah.com \
--to=greg@kroah.com \
--cc=James.Bottomley@suse.de \
--cc=akpm@linux-foundation.org \
--cc=bharrosh@panasas.com \
--cc=error27@gmail.com \
--cc=jirislaby@gmail.com \
--cc=kay.sievers@vrfy.org \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=segooon@gmail.com \
--cc=tj@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox