Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752791AbcKYXth convert rfc822-to-8bit (ORCPT ); Fri, 25 Nov 2016 18:49:37 -0500 Received: from mga01.intel.com ([192.55.52.88]:21864 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750859AbcKYXt3 (ORCPT ); Fri, 25 Nov 2016 18:49:29 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,697,1473145200"; d="scan'208";a="195705093" From: "Dilger, Andreas" To: James Simmons CC: Greg Kroah-Hartman , "devel@driverdev.osuosl.org" , "Drokin, Oleg" , Linux Kernel Mailing List , Lustre Development List Subject: Re: [lustre-devel] [PATCH 09/10] staging: lustre: libcfs: remove zero comparisons in headers Thread-Topic: [lustre-devel] [PATCH 09/10] staging: lustre: libcfs: remove zero comparisons in headers Thread-Index: AQHSQbunTk0Vx8bemU+Uf2G4vsaOhqDq77OA Date: Fri, 25 Nov 2016 23:49:16 +0000 Message-ID: <0DBDC329-9FB2-4749-9EC3-5B81901BA6D5@intel.com> References: <1479487724-20386-1-git-send-email-jsimmons@infradead.org> <1479487724-20386-10-git-send-email-jsimmons@infradead.org> In-Reply-To: <1479487724-20386-10-git-send-email-jsimmons@infradead.org> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.254.36.22] Content-Type: text/plain; charset="us-ascii" Content-ID: <4E9940FC64A9A04FAAE552FFB4602763@intel.com> Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1933 Lines: 43 On Nov 18, 2016, at 09:48, James Simmons wrote: > > Remove the zero comparisions in the libcfs headers. > > Signed-off-by: James Simmons > --- > .../lustre/include/linux/libcfs/libcfs_crypto.h | 2 +- > .../lustre/include/linux/libcfs/libcfs_fail.h | 4 +- > .../lustre/include/linux/libcfs/libcfs_hash.h | 32 ++++++++++---------- > 3 files changed, 19 insertions(+), 19 deletions(-) > > diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_crypto.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_crypto.h > index a0865e5..8f34c5d 100644 > --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_crypto.h > +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_crypto.h > @@ -137,7 +137,7 @@ static inline unsigned char cfs_crypto_hash_alg(const char *algname) > enum cfs_crypto_hash_alg hash_alg; > > for (hash_alg = 0; hash_alg < CFS_HASH_ALG_MAX; hash_alg++) > - if (strcmp(hash_types[hash_alg].cht_name, algname) == 0) > + if (!strcmp(hash_types[hash_alg].cht_name, algname)) I'd really rather keep the "== 0" comparison for strcmp(), because IMHO !strcmp() reads like "the strings do not compare the same" and is confusing. > static inline int > cfs_hash_is_iterating(struct cfs_hash *hs) > { > /* someone is calling cfs_hash_for_each_* */ > - return hs->hs_iterating || hs->hs_iterators != 0; > + return hs->hs_iterating || hs->hs_iterators; Likewise, I'd rather keep comparisons == 0 or != 0 for cases where the variable is an actual number instead of just a return code or a pointer. I don't think we need to be totally dogmatic about removing every comparison with 0 in all of the code, as that can reduce readability in some cases. I know this patch has already landed, and it isn't the end of the world either way, but just wanted to forestall similar changes being made through the rest of the code. Cheers, Andreas