Re: [RFC PATCH 1/4] PHY Abstraction Layer III (now with more splitiness)
From: Andy Fleming <hidden>
Date: 2005-07-27 18:46:08
Also in:
netdev
On Jul 27, 2005, at 13:08, Randy Dunlap wrote:
quoted
On Jul 25, 2005, at 16:06, Francois Romieu wrote:quoted
quoted
+int mdiobus_register(struct mii_bus *bus) +{ + int i; + int err = 0; + + spin_lock_init(&bus->mdio_lock); + + if (NULL == bus || NULL == bus->name || + NULL == bus->read || + NULL == bus->write)Be spartan: if (!bus || !bus->name || !bus->read || !bus->write)I think we have to agree to disagree here. I could be convinced, but I'm partial to using NULL explicitly.But there are 2 issues here (at least). One is to use NULL or not. The other is using (constant == var) or (var == constant). It's not described in CodingStlye afaik, but most recent email on the subject strongly prefers (var == constant) [in my unscientific survey -- of bits in my head]. So using the suggested style will fix both of these. :)
Ok, here I won't agree to disagree with you. !foo as a check for
NULL is a reasonable idea, but not my style. If that's the preferred
style for the kernel, I will do that.
But (var == constant) is a style that asks for errors. By putting
the constant first in these checks, you never run the risk of leaving
a bug like this:
if (dev = NULL)
...
This kind of error is quite frustrating to detect, and the eye will
often miss it when scanning for errors. If you follow constant ==
var, though, then the bug looks like this:
if (NULL = dev)
which is instantly caught by the compiler.
Just my 32 cents
quoted
quoted
quoted
+ /* Otherwise, we allocate the device, and initialize the + * default values */ + dev = kmalloc(sizeof(*dev), GFP_KERNEL); + + if (NULL == dev) { + errno = -ENOMEM; + return NULL; + } + + memset(dev, 0, sizeof(*dev));The kernel provides kcalloc.I went looking for it, and found it in fs/cifs/misc.c. I'm hesitant to link to a function defined in the filesystem code just to save 1 line of codeIt's more global than that.
Should we move the function, then, to include/linux/slab.h? Or somewhere else?