Chen Gang <cheng...@emindsoft.com.cn> writes:
> On 7/11/16 07:47, Dave Hansen wrote:
>> On 07/09/2016 09:29 AM, cheng...@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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
