From: Lars-Peter Clausen <lars@metafoo.de>
To: Jean Delvare <khali@linux-fr.org>
Cc: Wolfram Sang <wsa@the-dreams.de>, Ben Dooks <ben-linux@fluff.org>,
linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/6] Make return type of i2c_del_adapter() (and i2c_del_mux_adapter()) void
Date: Sat, 30 Mar 2013 21:43:29 +0100 [thread overview]
Message-ID: <51574E71.7010403@metafoo.de> (raw)
In-Reply-To: <20130330211352.07530db7@endymion.delvare>
On 03/30/2013 09:13 PM, Jean Delvare wrote:
> Hi Lars,
>
> On Sat, 9 Mar 2013 19:16:43 +0100, Lars-Peter Clausen wrote:
>> Currently i2c_del_adapter() returns 0 on success and potentially an error code
>> on failure. Unfortunately this doesn't mix too well with the Linux device driver
>> model. (...)
>
> I see:
>
> struct device_driver {
> (...)
> int (*probe) (struct device *dev);
> int (*remove) (struct device *dev);
>
> So the driver core does allow remove functions to return an error.
Well, the return type is int, but the return value is never checked. So you
can return an error value, but the device driver core is going to care and
the device is still removed. So any code which does return an error in it's
probe function in the assumption that this means the device will still be
present is broken and leaves the system in an undefined state. So if that
happens the kernel will probably crash sooner or later, or if you are lucky
you only created a few resources leaks.
And no we can't change the core to handle errors from a drivers remove
callback. It's a basic inherent property of the Linux device driver model
that any device can be removed at any time.
> Are you going to fix all subsystems as you are doing for i2c now, and then
> change device_driver.remove to return void? If not, I don't see the
> point of changing it in i2c.
>
As I said it's a bug if a driver returns an error in its remove function.
And the fact that the return type of the remove callback is int is pretty
much misleading in this regard, so the long term goal is to make the return
type void. But that's a long way to go until we get there, fixing the return
type of i2c_del_adapter() is kind of the low hanging fruit.
- Lars
next prev parent reply other threads:[~2013-03-30 20:43 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-09 18:16 [PATCH 0/6] Make return type of i2c_del_adapter() (and i2c_del_mux_adapter()) void Lars-Peter Clausen
2013-03-09 18:16 ` [PATCH 1/6] i2c: Remove detach_adapter Lars-Peter Clausen
[not found] ` <1362853009-20789-2-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-30 14:52 ` Jean Delvare
2013-03-09 18:16 ` [PATCH 2/6] i2c: i2c_del_adapter: Don't treat removing a non-registered adapter as error Lars-Peter Clausen
[not found] ` <1362853009-20789-3-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-30 18:17 ` Jean Delvare
2013-03-09 18:16 ` [PATCH 3/6] i2c: Ignore return value of i2c_del_adapter() Lars-Peter Clausen
[not found] ` <1362853009-20789-4-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-11 17:52 ` Ben Hutchings
2013-03-30 6:34 ` Wolfram Sang
2013-03-30 7:55 ` Jean Delvare
[not found] ` <20130330085539.442321fa-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2013-03-30 8:11 ` Lars-Peter Clausen
[not found] ` <51569E4A.2080006-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-30 8:43 ` Jean Delvare
2013-03-31 13:12 ` Shawn Guo
2013-03-31 8:25 ` Jean Delvare
2013-03-09 18:16 ` [PATCH 4/6] i2c: Make return type of i2c_del_adapter() void Lars-Peter Clausen
[not found] ` <1362853009-20789-5-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-31 9:04 ` Jean Delvare
2013-03-09 18:16 ` [PATCH 5/6] i2c: Ignore the return value of i2c_del_mux_adapter() Lars-Peter Clausen
[not found] ` <1362853009-20789-6-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-13 6:05 ` [5/6] " Guenter Roeck
2013-03-31 9:23 ` [PATCH 5/6] " Jean Delvare
2013-03-09 18:16 ` [PATCH 6/6] i2c: Make the return type of i2c_del_mux_adapter() void Lars-Peter Clausen
[not found] ` <1362853009-20789-7-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-31 9:24 ` Jean Delvare
2013-03-30 6:33 ` [PATCH 0/6] Make return type of i2c_del_adapter() (and i2c_del_mux_adapter()) void Wolfram Sang
[not found] ` <1362853009-20789-1-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-30 20:13 ` Jean Delvare
2013-03-30 20:43 ` Lars-Peter Clausen [this message]
[not found] ` <51574E71.7010403-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-31 7:56 ` Jean Delvare
2013-03-31 11:30 ` Jean Delvare
2013-04-02 5:09 ` Wolfram Sang
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=51574E71.7010403@metafoo.de \
--to=lars@metafoo.de \
--cc=ben-linux@fluff.org \
--cc=khali@linux-fr.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=wsa@the-dreams.de \
/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;
as well as URLs for NNTP newsgroup(s).