i2c_use_client() defends against NULL
Subsystems (2)
PATCH 1/1 i2c_use_client() defends against NULL
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.
--- drivers/i2c/i2c-core.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/i2c/i2c-core.c +5 -3
@@ -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
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c
A message that is not in the archive (sent off-list or not yet received): 200805041306.21074.david-b-yBeKhBN/[email protected]
Hi David,
Quoted 3 lines · 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.
Passing NULL to the current functions would just crash, it wouldn't morph anything.
Quoted 2 lines
Note that none of the current
callers ofi2c_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.
Quoted 21 lines
Signed-off-by David Brownell <[email protected]>--- drivers/i2c/i2c-core.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)drivers/i2c/i2c-core.c +3
-2@@ -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);
drivers/i2c/i2c-core.c EXPORT_SYMBOL(i2c_use_client); · line 1034
Quoted 9 lines
drivers/i2c/i2c-core.c +2
-1@@ -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,
Signature · 6 lines
Jean Delvare _______________________________________________ i2c mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/i2c
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 7 lines · Jean Delvare
Quoted 3 lines · 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.
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
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c
A message that is not in the archive (sent off-list or not yet received): 200805170657.56833.david-b-yBeKhBN/[email protected]
Quoted 11 lines · David Brownell
Quoted 7 lines · Jean Delvare
Quoted 3 lines · 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.
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
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c
A message that is not in the archive (sent off-list or not yet received): Pine.LNX.4.58.0805171048070.4196-13q4cmjDBaTP3RPoUHIrnuTW4wlIGRCZ@public.gmane.org
Quoted 8 lines · Trent Piepho
Quoted 5 lines
Quoted 2 lines
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
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c