Re: [PATCH v2] driver core: let dev_set_drvdata return int insteadof void as it can fail

From: Greg KH
Date: Wed Apr 20 2011 - 13:20:48 EST


On Wed, Apr 20, 2011 at 11:09:56AM +0200, MichaÅ MirosÅaw wrote:
> 2011/4/20 Greg KH <greg@xxxxxxxxx>:
> > On Mon, Apr 11, 2011 at 08:42:58PM +0200, Uwe Kleine-KÃnig wrote:
> >> Before commit
> >>
> >> Â Â Â b402843 (Driver core: move dev_get/set_drvdata to drivers/base/dd.c)
> >>
> >> calling dev_set_drvdata with dev=NULL was an unchecked error. After some
> >> discussion about what to return in this case removing the check (and so
> >> producing a null pointer exception) seems fine.
> > I'm confused by this thread, care to resend all of these in a series
> > against the latest linux-next tree?
>
> I'd argue that dev_set_drvdata() should never fail. All current
> drivers depend on this, and if dev_set_drvdata() fails, user will get
> an OOPS a short while after the device finishes initializing (or maybe
> even before that if callbacks are involved).
> Allowing dev_set_drvdata() to fail will need putting a lot of
> boilerplate code into drivers for no real gain.
>
> Please consider reverting commit
> b4028437876866aba4747a655ede00f892089e14 instead of "fixing" issues it
> generates.

That patch was from 2009, surely if there were real issues with that
change, it would have shown up in the past 2 years, right?

And no, I don't want to revert that, we need that for future work in
this area.

I have no problem migrating the error code for that function on down,
very few drivers call this function directly, it should be wrapped by
bus-specific functions instead, right? They can handle the error
handling on their own and not force the individual drivers to handle it
if needed.

Have you ever seen this function fail?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/