On 06/15/2010 04:40 AM, Jean Delvare wrote:
Hi David,
On Mon, 14 Jun 2010 14:28:57 -0700, David Daney wrote:
On 06/14/2010 01:53 PM, Jean Delvare wrote:
Hi Justin,
On Mon, 14 Jun 2010 13:26:46 -0700, Justin P. Mattock wrote:
could be a right solution, could be wrong here is the warning: CC drivers/i2c/i2c-core.o drivers/i2c/i2c-core.c: In function 'i2c_register_adapter': drivers/i2c/i2c-core.c:757:15: warning: variable 'dummy' set but not used
Signed-off-by: Justin P. Mattockjustinmattock@gmail.com
drivers/i2c/i2c-core.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 1cca263..79c6c26 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -794,6 +794,8 @@ static int i2c_register_adapter(struct i2c_adapter *adap) mutex_lock(&core_lock); dummy = bus_for_each_drv(&i2c_bus_type, NULL, adap, __process_new_adapter);
- if(!dummy)
dummy = 0;
One word: scripts/checkpatch.pl
In other news, the above is just plain wrong. First we force people to read the result of bus_for_each_drv() and then when they do and don't need the value, gcc complains, so we add one more layer of useless code, which developers and possibly tools will later wonder and complain about? I can easily imagine that a static code analyzer would spot the above code as being a potential bug.
Let's stop this madness now please.
Either __must_check goes away from bus_for_each_drv() and from every other function which raises this problem, or we must disable that new type of warning gcc 4.6.0 generates. Depends which warnings we value more, as we can't sanely have both.
That is the crux of the whole thing. Putting in crap to get rid of the __must_check warning someone obviously wanted to provoke is just plain wrong.
__process_new_adapter() calls i2c_do_add_adapter() which always returns 0. Why should I check the return value of bus_for_each_drv() when I know it will always be 0 by construction?
Also note that the same function is also called through bus_for_each_dev() somewhere else in i2c-core, and there is no warning there because bus_for_each_dev() is not marked __must_check. How consistent is this? If bus_for_each_dev() is OK without __must_check, then I can't see why bus_for_each_drv() wouldn't be.
Well, I would advocate removing the __must_check then.
David Daney
On Tue, 15 Jun 2010 09:20:39 -0700, David Daney wrote:
On 06/15/2010 04:40 AM, Jean Delvare wrote:
__process_new_adapter() calls i2c_do_add_adapter() which always returns 0. Why should I check the return value of bus_for_each_drv() when I know it will always be 0 by construction?
Also note that the same function is also called through bus_for_each_dev() somewhere else in i2c-core, and there is no warning there because bus_for_each_dev() is not marked __must_check. How consistent is this? If bus_for_each_dev() is OK without __must_check, then I can't see why bus_for_each_drv() wouldn't be.
Well, I would advocate removing the __must_check then.
I have just sent a patch to LKML doing exactly this.
dri-devel@lists.freedesktop.org