From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1758656AbbLBOG2 (ORCPT <rfc822;w@1wt.eu>);
	Wed, 2 Dec 2015 09:06:28 -0500
Received: from userp1040.oracle.com ([156.151.31.81]:27893 "EHLO
	userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1758344AbbLBOAN (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Wed, 2 Dec 2015 09:00:13 -0500
Date: Wed, 2 Dec 2015 16:59:30 +0300
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
Message-ID: <20151202135930.GM7289@mwanda>
References: <1448062576-23757-1-git-send-email-jsimmons@infradead.org>
 <1448062576-23757-22-git-send-email-jsimmons@infradead.org>
 <20151202125440.GS18797@mwanda>
 <CAHtKf-aEa4YvhgvZgAFLuOVbX=bdmC+YQwaAH5p2-qCAS_W8Vg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAHtKf-aEa4YvhgvZgAFLuOVbX=bdmC+YQwaAH5p2-qCAS_W8Vg@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
X-Source-IP: userv0021.oracle.com [156.151.31.71]
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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