Re: [announce] new tree: "fix all build warnings, on all configs"

From: Ingo Molnar
Date: Wed Oct 22 2008 - 05:49:17 EST



* Jörn Engel <joern@xxxxxxxxx> wrote:

> On Mon, 20 October 2008 21:21:10 +0200, Ingo Molnar wrote:
> >
> > /* Alas, no aliases. Too much hassle with bringing module.h everywhere */
> > #define fops_get(fops) \
> > - (((fops) && try_module_get((fops)->owner) ? (fops) : NULL))
> > + (((fops != NULL) && try_module_get((fops)->owner) ? (fops) : NULL))
> > #define fops_put(fops) \
> > - do { if (fops) module_put((fops)->owner); } while(0)
> > + do { if (fops != NULL) module_put((fops)->owner); } while(0)
>
> This, I would argue, makes the code worse.

Have a look at:

$ git log -p --grep="NULL noise"

for example:

for (i = 0; i < MAX_FEB_SIZE; i++)
- if (tb->FEB[i] != 0)
+ if (tb->FEB[i] != NULL)
break;

so checking for != NULL is a valid way of testing a pointer's existence.
The "if (tb->FEB[i])" is a valid shortcut for the same thing as well.

In this specific case the issue is that the 'fops' parameter can
occasionally be a constant pointer (turning the test into always-true)
so the compiler is at least minimally correct at asking the "are you
sure you want this" question - which we answer in the affirmative via
the explicit NULL check. But these are really nuances.

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/