On Jul 27, 2005, at 13:08, Randy Dunlap wrote:



On Jul 25, 2005, at 16:06, Francois Romieu wrote:



+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




+    /* 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 code


It's more global than that.


Should we move the function, then, to include/linux/slab.h? Or somewhere else?

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to