Re: [lustre-devel] [PATCH 21/40] staging: lustre: improve LNet clean up code and API

From	Dan Carpenter <dan.carpenter@oracle.com>
Newsgroups	linux.kernel
Subject	Re: [lustre-devel] [PATCH 21/40] staging: lustre: improve LNet clean up code and API
Date	2015-12-02 15:10 +0100
Message-ID	<qBggz-8db-29@gated-at.bofh.it> (permalink)
References	<qx3rz-5CJ-5@gated-at.bofh.it> <qx3Bg-5Gz-11@gated-at.bofh.it> <qBfkt-7B5-13@gated-at.bofh.it> <qBg6T-7Ug-27@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Wed, Dec 02, 2015 at 04:20:59PM +0300, Alexander Zarochentsev wrote:
> >  BAD:   if (rc != 0)
> > GOOD:   if (rc)
> 
> The latest suggestion is not correct,
> from http://wiki.lustre.org/Lustre_Coding_Guidelines :
> Conditional boolean (if (expr)), scalar (if (val != 0)) and pointer
> (if (ptr != NULL)) expressions should be written consistently.

Kernel style trumps Lustre style.  Double negative don't not hurt
readability.  != NULL is a checkpatch.pl warning.  Also comparisons like
== false or == true are checkpatch warnings.

I can think of two places where comparing with zero is appropriate and
those are:

1)  If you are talking about the numer zero.

	if (x == 0 || x == 2)

2) strcmp() and other *cmp() functions.

	if (strcmp(foo, bar) == 0)   /* foo and bar are the same */
	if (strcmp(foo, bar) < 0)    /* foo less than bar */
	if (strcmp(foo, bar) != 0)   /* foo not the same as bar */

For "if (rc) {" a zero return doesn't mean zero, it means success so
comparing against zero is bad style.

regards,
dan carpenter

--
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/

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: [PATCH 21/40] staging: lustre: improve LNet clean up code and API Dan Carpenter <dan.carpenter@oracle.com> - 2015-12-02 14:10 +0100
  Re: [lustre-devel] [PATCH 21/40] staging: lustre: improve LNet clean  up code and API Alexander Zarochentsev <alexander.zarochentsev@seagate.com> - 2015-12-02 15:00 +0100
    Re: [lustre-devel] [PATCH 21/40] staging: lustre: improve LNet clean  up code and API Dan Carpenter <dan.carpenter@oracle.com> - 2015-12-02 15:10 +0100

csiph-web