From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
To: David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
Cc: i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
Subject: Re: [patch 2.6.26-rc1] i2c_use_client() defends against NULL
Date: Sat, 17 May 2008 14:40:07 +0200 [thread overview]
Message-ID: <20080517144007.7611ab32@hyperion.delvare> (raw)
In-Reply-To: <200805041306.21074.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
Hi David,
On Sun, 4 May 2008 13:06:20 -0700, David Brownell wrote:
> Defend the i2c refcount calls against NULL pointers, as is important
> (and conventional) for such calls ... we don't want to morph NULL
> pointers into bogus non-null ones.
Passing NULL to the current functions would just crash, it wouldn't
morph anything.
> Note that none of the current
> callers of i2c_use_client() use its return value.
Which sounds sane... when a function can only fail if you give it a
bogus parameter, it makes more sense to check the parameter for
validity yourself than to check the value returned by the function
afterwards. I'm not even sure why these functions return something in
the first place.
>
> Signed-off-by: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> ---
> drivers/i2c/i2c-core.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> --- g26.orig/drivers/i2c/i2c-core.c 2008-05-04 12:50:33.000000000 -0700
> +++ g26/drivers/i2c/i2c-core.c 2008-05-04 12:51:04.000000000 -0700
> @@ -1013,8 +1013,9 @@ EXPORT_SYMBOL(i2c_detach_client);
> */
> struct i2c_client *i2c_use_client(struct i2c_client *client)
> {
> - get_device(&client->dev);
> - return client;
> + if (client && get_device(&client->dev))
> + return client;
> + return NULL;
> }
> EXPORT_SYMBOL(i2c_use_client);
>
> @@ -1026,7 +1027,8 @@ EXPORT_SYMBOL(i2c_use_client);
> */
> void i2c_release_client(struct i2c_client *client)
> {
> - put_device(&client->dev);
> + if (client)
> + put_device(&client->dev);
> }
> EXPORT_SYMBOL(i2c_release_client);
>
I hate this defensive programming, to me it's a waste of CPU cycles.
But as you said, it's apparently the norm in the Linux kernel to do
these useless checks for that specific type of functions, so I'll take
your patch even though I don't like it.
Thanks,
--
Jean Delvare
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
next prev parent reply other threads:[~2008-05-17 12:40 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-05-04 20:06 [patch 2.6.26-rc1] i2c_use_client() defends against NULL David Brownell
[not found] ` <200805041306.21074.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-05-17 12:40 ` Jean Delvare [this message]
[not found] ` <20080517144007.7611ab32-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2008-05-17 13:57 ` David Brownell
[not found] ` <200805170657.56833.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-05-17 17:53 ` Trent Piepho
[not found] ` <Pine.LNX.4.58.0805171048070.4196-13q4cmjDBaTP3RPoUHIrnuTW4wlIGRCZ@public.gmane.org>
2008-05-17 18:53 ` David Brownell
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=20080517144007.7611ab32@hyperion.delvare \
--to=khali-puyad+kwke1g9huczpvpmw@public.gmane.org \
--cc=david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org \
--cc=i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.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