Messages in this thread |  | | Date | Sat, 3 Feb 2007 16:40:42 -0800 | From | Randy Dunlap <> | Subject | Re: A CodingStyle suggestion |
| |
On Sat, 03 Feb 2007 16:21:18 -0800 Roland Dreier wrote:
> > Good catch :). A small grep of `access_ok' reveals that it's always used in the > > form of: > > if (!access_ok()) { .. } > > > > I can conclude that verbal/imperative methods like `kmalloc, add_work' be > > checked as: > > ret = do_work(); > > if (ret) { ... } > > and predicate methods like `acess_ok, pci_dev_present' be checked like: > > if (!access_ok) { ... } > > if (pci_dev_present) { ...} > > > > Any comments ? > > I don't think that's really the distinction that matters. I think > really the issue is that assignment within an if is hard to read, so > > ret = foo(a, b); > if (ret) { ... } > > is clearly preferred to > > if ((ret = foo(a,b))) { ... } > > However, in my opinion something like > > if (foo(a,b)) { ... } > > if perfectly fine if the return value of foo is not needed anywhere > else. In other words, there's no sense introducing a temporary > variable to hold the return value if you're never going to do anything > with it other than check it on the next line.
I agree with Roland's comments here.
And with Tim's about side effects.
--- ~Randy - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |