[PATCH] include: mman: Use bool instead of int for the return value of arch_validate_prot

Michael Ellerman mpe at ellerman.id.au
Tue Jul 12 14:20:08 AEST 2016


Chen Gang <chengang at emindsoft.com.cn> writes:

> On 7/11/16 07:47, Dave Hansen wrote:
>> On 07/09/2016 09:29 AM, chengang at 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.

cheers


More information about the Linuxppc-dev mailing list