From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754774AbcGLQre (ORCPT ); Tue, 12 Jul 2016 12:47:34 -0400 Received: from out1134-226.mail.aliyun.com ([42.120.134.226]:27966 "EHLO out1134-226.mail.aliyun.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751193AbcGLQrd (ORCPT ); Tue, 12 Jul 2016 12:47:33 -0400 X-Alimail-AntiSpam: AC=CONTINUE;BC=0.07477459|-1;FP=0|0|0|0|0|-1|-1|-1;HT=e02c03296;MF=chengang@emindsoft.com.cn;NM=1;PH=DS;RN=10;RT=10;SR=0;TI=SMTPD_----50YCHiG_1468342039; Message-ID: <57852093.1090909@emindsoft.com.cn> Date: Wed, 13 Jul 2016 00:53:39 +0800 From: Chen Gang User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Michael Ellerman , Dave Hansen , akpm@linux-foundation.org, benh@kernel.crashing.org, paulus@samba.org CC: tglx@linutronix.de, mingo@kernel.org, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, Chen Gang Subject: Re: [PATCH] include: mman: Use bool instead of int for the return value of arch_validate_prot References: <1468081751-9468-1-git-send-email-chengang@emindsoft.com.cn> <5782DEA5.600@linux.intel.com> <5783ED17.9010805@emindsoft.com.cn> <878tx7cwsn.fsf@@concordia.ellerman.id.au> In-Reply-To: <878tx7cwsn.fsf@@concordia.ellerman.id.au> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 7/12/16 12:20, Michael Ellerman wrote: > Chen Gang writes: > >> On 7/11/16 07:47, Dave Hansen wrote: >>> On 07/09/2016 09:29 AM, chengang@emindsoft.com.cn wrote: >>>> -static inline int arch_validate_prot(unsigned long prot) >>>> +static inline bool arch_validate_prot(unsigned long prot) >>>> { >>>> if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_SAO)) >>>> - return 0; >>>> - if ((prot & PROT_SAO) && !cpu_has_feature(CPU_FTR_SAO)) >>>> - return 0; >>>> - return 1; >>>> + return false; >>>> + return (prot & PROT_SAO) == 0 || cpu_has_feature(CPU_FTR_SAO); >>>> } >>>> #define arch_validate_prot(prot) arch_validate_prot(prot) >>> >>> Please don't do things like this. They're not obviously correct and >>> also have no obvious benefit. You also don't mention why you bothered >>> to alter the logical structure of these checks. >>> >> >> For all cases, bool is equal or a little better than int, and they are >> equal in our case (2 final outputs are same). So for me, it may belong >> to trivial patch, which can be skipped by the normal patch maintainers. >> >> As a 'trivial' patch: >> >> - For a pure Boolean function, bool return value is more readable than >> int. > > Agreed. > > Please send a patch that does that and only that. > OK, thanks. After check the assembly output, for some cases, merging 3 lines to 1 line may be a little more readable, but compiler will generate a little bad output code. I shall send patch v2 for it within this weekend. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.