Re: PATCH, RFC: 2.6 Documentation/Codingstyle

From: Kevin O'Connor (kevin_at_koconnor.net)
Date: 02/14/04

  • Next message: Randy.Dunlap: "[PATCH] syscalls.h update #9 (open/close)" 
    Date:	Fri, 13 Feb 2004 19:38:21 -0500
To: Michael Frank <mhf@linuxmail.org>

    On Fri, Feb 13, 2004 at 06:15:10AM +0800, Michael Frank wrote:
    > +int fun(int )
    > +{
    > + int result = 0;
    > + char *buffer = kmalloc(SIZE);
    > +
    > + if (buffer == 0) {
    > + result = -1;
    > + goto out;
    > + }
    > +
    > + if (condition1) {
    > + while (loop1) {
    > + ...
    > + }
    > + result = 1;
    > + goto out;
    > + }
    > + if (condition2) {
    > + while (loop2) {
    > + ...
    > + }
    > + result = 2;
    > + goto out;
    > + }
    > +out:
    > + if (buffer)
    > + kfree(buffer);
    > + return result;
    > +}

    This is a bit of a nitpick, but if you're going to give a code example in a
    file titled "Coding style" I'd recommend two changes:

    1 - kfree already checks for null, so checking for null before calling
        kfree is pointless.

    2 - I personally think testing pointers against an integer is bad style, so
        I'd prefer to see the original test changed to "if (!buffer)" or
        less-preferably "if (buffer == NULL)".

    > +Macros with multiple statements should be enclosed in a do - while block.
    > +
    > +#define macrofun(a,b,c) \
    > +do { \
    > + if (a == 5) \
    > + do_this(b,c); \
    > + \
    > +} while (0)

    Enclosing macros in a do-while block is sound advice, but the above is an
    example of something that should be an inline function.

    -Kevin 

    -- 
 ---------------------------------------------------------------------
 | Kevin O'Connor                  "BTW, IMHO we need a FAQ for      |
 | kevin@koconnor.net               'IMHO', 'FAQ', 'BTW', etc. !"    |
 ---------------------------------------------------------------------
-
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/
  • Next message: Randy.Dunlap: "[PATCH] syscalls.h update #9 (open/close)"

    Relevant Pages