[patch 2.6.26-rc1] i2c_use_client() defends against NULL

[patch 2.6.26-rc1] i2c_use_client() defends against NULL

[David Brownell]

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.  Note that none of the current
callers of i2c_use_client() use its return value.

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);
 

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [patch 2.6.26-rc1] i2c_use_client() defends against NULL

[Jean Delvare]

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

Re: [patch 2.6.26-rc1] i2c_use_client() defends against NULL

[David Brownell]

On Saturday 17 May 2008, Jean Delvare wrote:
> 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.

That's only if you assume an MMU set up to cause oopsing on reference
to the first and last pages of memory.  Not the best of assumptions...

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [patch 2.6.26-rc1] i2c_use_client() defends against NULL

[Trent Piepho]

On Sat, 17 May 2008, David Brownell wrote:
> On Saturday 17 May 2008, Jean Delvare wrote:
> > 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.
>
> That's only if you assume an MMU set up to cause oopsing on reference
> to the first and last pages of memory.  Not the best of assumptions...

Maybe BUG_ON(!client) would be better?  It seems more likely to catch
a programming error.

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [patch 2.6.26-rc1] i2c_use_client() defends against NULL

[David Brownell]

On Saturday 17 May 2008, Trent Piepho wrote:
> > > Passing NULL to the current functions would just crash, it wouldn't
> > > morph anything.
> >
> > That's only if you assume an MMU set up to cause oopsing on reference
> > to the first and last pages of memory.  Not the best of assumptions...
> 
> Maybe BUG_ON(!client) would be better?  It seems more likely to catch
> a programming error.

BUG_ON() is almost always the wrong answer though ... it's only
appropriate when there's no way for the system to continue.

And in fact, here it's *not* a bug...

Passing NULL around is more often OK than not.  Example, it's
expected that passing it to a "free this object" routine will
work, saving callers a NULL check.  Similarly, refcount calls.


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c