Re: [PATCH] 2.4.4-ac11 network drivers cleaning

From: Jeff Garzik (jgarzik@mandrakesoft.com)
Date: Sat May 19 2001 - 17:58:49 EDT

  • Next message: Ben Ford: "Re: CML2 design philosophy heads-up"

    Patch looks decent. Adding module descriptions was quite nice. One
    flaw that is repeated multiple times is that you add

            #ifdef MODULE
            printk(version);
            #endif

    in an ISA driver's probe routine. This instead should always be the
    first operation of init_module.

    Also make sure to go through the 'ac' patch and review the earlier
    version string changes. Some of them were buggy, like

            static const char version[] __initdata =
            "...";

    const combined with __[dev]initdata causes a section type conflict. A
    few of those popped up after the earlier patch was applied to 'ac'.

    Finally, I don't know if I mentioned this earlier, but to be complete
    and optimal, version strings should be a single variable 'version', such
    that it can be passed directly to printk like

            printk(version);

    Some net drivers are already like this, as I'm sure you know. Some net
    drivers have 'version', 'version2', 'version3' instead of just one long
    string. Some net drivers add KERN_xxx at printk time, instead of adding
    it to the 'version' var. Some net drivers do the following, which is
    really silly considering you know all strings at compile time:

            printk(KERN_INFO "%s\n", version);

    -- 
    Jeff Garzik      | "Do you have to make light of everything?!"
    Building 1024    | "I'm extremely serious about nailing your
    MandrakeSoft     |  step-daughter, but other than that, yes."
    -
    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/
    



    This archive was generated by hypermail 2b29 : Sat May 19 2001 - 18:03:09 EDT