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. Mattock<justinmattock at 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
