Re: [PATCH 02/16] ehca: module infrastructure

From: Jörn Engel
Date: Thu Apr 27 2006 - 08:09:41 EST


On Thu, 27 April 2006 12:48:05 +0200, Heiko J Schick wrote:
>
> + if (ehca_module->cache_pd == NULL) {

Hmm.

> + ret = kmem_cache_destroy(ehca_module->cache_pd);
> + if (ret != 0)

The " != 0" is completely superfluous. Above NULL check is a matter
of taste, this one demonstates lack of boolean algebra understanding.

> + rblock = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
> + if (!rblock) {

Hmm. And your taste seems to change. :)

> + if (ehca_hw_level == 0) {

And since we're on the subject. Ignoring the recent discussion
involving akpm, viro and others, the kernel historically used int both
for integer and boolean, plus return values as a special kind of
"boolean with error indication attached".

For boolean, it is nicer to do things like "if (!error)", for
integers, a comparison as above is nicer. Return codes fall into the
boolean category. Pointers after kmalloc() and similar are viewed as
rich boolean by some people, but not by all.

Jörn

--
Geld macht nicht glücklich.
Glück macht nicht satt.
-
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/