Re: PATCH, RFC: 2.6 Documentation/Codingstyle

[Kevin O'Connor]

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@xxxxxxxxxxxx               'IMHO', 'FAQ', 'BTW', etc. !"    |
 ---------------------------------------------------------------------
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/