public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Alexander Zarochentsev <alexander.zarochentsev@seagate.com>
Cc: James Simmons <jsimmons@infradead.org>,
	devel@driverdev.osuosl.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Oleg Drokin <oleg.drokin@intel.com>,
	Amir Shehata <amir.shehata@intel.com>,
	lustre-devel@lists.lustre.org
Subject: Re: [lustre-devel] [PATCH 21/40] staging: lustre: improve LNet clean up code and API
Date: Wed, 2 Dec 2015 16:59:30 +0300	[thread overview]
Message-ID: <20151202135930.GM7289@mwanda> (raw)
In-Reply-To: <CAHtKf-aEa4YvhgvZgAFLuOVbX=bdmC+YQwaAH5p2-qCAS_W8Vg@mail.gmail.com>

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


  reply	other threads:[~2015-12-02 14:06 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-20 23:35 [PATCH 00/40] Sync upstream lustre client LNet core James Simmons
2015-11-20 23:35 ` [PATCH 01/40] staging: lustre: drop *_t from end of struct lnet_text_buf James Simmons
2015-11-20 23:35 ` [PATCH 02/40] staging: lustre: fix 'NULL pointer dereference' errors for LNet James Simmons
2015-12-02  7:46   ` Dan Carpenter
2015-12-15 18:08     ` Simmons, James A.
2015-11-20 23:35 ` [PATCH 03/40] staging: lustre: reflect down routes in /proc/sys/lnet/routes James Simmons
2015-12-02  7:54   ` Dan Carpenter
2015-11-20 23:35 ` [PATCH 04/40] staging: lustre: fix failure handle of create reply James Simmons
2015-11-20 23:35 ` [PATCH 05/40] staging: lustre: eliminate obsolete Cray SeaStar support James Simmons
2015-11-20 23:35 ` [PATCH 06/40] staging: lustre: remove uses of IS_ERR_VALUE() James Simmons
2015-11-21 18:45   ` Dan Carpenter
2015-11-20 23:35 ` [PATCH 07/40] staging: lustre: return +ve for blocked lnet message James Simmons
2015-11-20 23:35 ` [PATCH 08/40] staging: lustre: do not memset after LIBCFS_ALLOC James Simmons
2015-11-20 23:35 ` [PATCH 09/40] staging: lustre: Dynamic LNet Configuration (DLC) James Simmons
2015-11-20 23:35 ` [PATCH 10/40] staging: lustre: Dynamic LNet Configuration (DLC) dynamic routing James Simmons
2015-11-20 23:35 ` [PATCH 11/40] staging: lustre: DLC Feature dynamic net config James Simmons
2015-12-02  9:23   ` Dan Carpenter
2015-11-20 23:35 ` [PATCH 12/40] staging: lustre: Dynamic LNet Configuration (DLC) IOCTL changes James Simmons
2015-12-02  9:48   ` Dan Carpenter
2015-11-20 23:35 ` [PATCH 13/40] staging: lustre: Dynamic LNet Configuration (DLC) show command James Simmons
2015-12-02 11:20   ` Dan Carpenter
2015-12-15 18:14     ` Simmons, James A.
2015-12-15 18:19       ` Dan Carpenter
2015-12-15 18:39         ` Simmons, James A.
2015-12-15 18:48       ` Greg Kroah-Hartman
2015-12-15 19:48         ` Simmons, James A.
2015-12-15 19:55           ` 'Greg Kroah-Hartman'
2015-12-02 12:00   ` Dan Carpenter
2015-11-20 23:35 ` [PATCH 14/40] staging: lustre: fix crash due to NULL networks string James Simmons
2015-12-02 11:27   ` Dan Carpenter
2015-11-20 23:35 ` [PATCH 15/40] staging: lustre: DLC user/kernel space glue code James Simmons
2015-12-02 12:11   ` Dan Carpenter
2015-11-20 23:35 ` [PATCH 16/40] staging: lustre: make local functions static for LNet ni James Simmons
2015-11-20 23:35 ` [PATCH 17/40] staging: lustre: add sparse annotation __user wherever needed for lnet James Simmons
2015-11-20 23:35 ` [PATCH 18/40] staging: lustre: remove LUSTRE_{,SRV_}LNET_PID James Simmons
2015-11-20 23:35 ` [PATCH 19/40] staging: lustre: copy out libcfs ioctl inline buffer James Simmons
2015-12-02 12:34   ` Dan Carpenter
2015-11-20 23:35 ` [PATCH 20/40] staging: lustre: fix kernel crash when network failed to start James Simmons
2015-12-02 12:44   ` Dan Carpenter
2015-11-20 23:35 ` [PATCH 21/40] staging: lustre: improve LNet clean up code and API James Simmons
2015-12-02 12:59   ` Dan Carpenter
2015-12-02 13:20     ` [lustre-devel] " Alexander Zarochentsev
2015-12-02 13:59       ` Dan Carpenter [this message]
2015-12-15 17:10     ` Simmons, James A.
2015-12-15 17:41       ` Dan Carpenter
2015-11-20 23:35 ` [PATCH 22/40] staging: lustre: Fixes to make lnetctl function as expected James Simmons
2015-11-20 23:35 ` [PATCH 23/40] staging: lustre: return appropriate errno when adding route James Simmons
2015-11-20 23:36 ` [PATCH 24/40] staging: lustre: make some lnet functions static James Simmons
2015-11-20 23:36 ` [PATCH 25/40] staging: lustre: missed a few cases of using NULL instead of 0 James Simmons
2015-11-20 23:36 ` [PATCH 26/40] staging: lustre: startup lnet acceptor thread dynamically James Simmons
2015-11-20 23:36 ` [PATCH 27/40] staging: lustre: reject invalid net configuration for lnet James Simmons
2015-11-20 23:36 ` [PATCH 28/40] staging: lustre: return -EEXIST if NI is not unique James Simmons
2015-11-20 23:36 ` [PATCH 29/40] staging: lustre: handle lnet_check_routes() errors James Simmons
2015-11-20 23:36 ` [PATCH 30/40] staging: lustre: improvement to router checker James Simmons
2015-11-20 23:36 ` [PATCH 31/40] staging: lustre: assume a kernel build James Simmons
2015-11-20 23:36 ` [PATCH 32/40] staging: lustre: prevent assert on LNet module unload James Simmons
2015-11-20 23:36 ` [PATCH 33/40] staging: lustre: remove messages from lazy portal on NI shutdown James Simmons
2015-11-20 23:36 ` [PATCH 34/40] staging: lustre: remove unnecessary EXPORT_SYMBOL from lnet layer James Simmons
2015-11-20 23:36 ` [PATCH 35/40] staging: lustre: avoid race during lnet acceptor thread termination James Simmons
2015-11-20 23:36 ` [PATCH 36/40] staging: lustre: test for sk_sleep presence in compact-2.6.h James Simmons
2015-11-20 23:36 ` [PATCH 37/40] staging: lustre: remove unnecessary NULL check in IOC_LIBCFS_GET_NET James Simmons
2015-11-20 23:36 ` [PATCH 38/40] staging: lustre: Allocate the correct number of rtr buffers James Simmons
2015-11-20 23:36 ` [PATCH 39/40] staging: lustre: Use lnet_is_route_alive for router aliveness James Simmons
2015-11-20 23:36 ` [PATCH 40/40] staging: lustre: Remove LASSERTS from router checker James Simmons
2015-12-21 23:41 ` [PATCH 00/40] Sync upstream lustre client LNet core Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151202135930.GM7289@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=alexander.zarochentsev@seagate.com \
    --cc=amir.shehata@intel.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jsimmons@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lustre-devel@lists.lustre.org \
    --cc=oleg.drokin@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox