public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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

  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