Re: [PATCH 1/3] powerpc/kernel: Remove nested if statements in rtas_initialize()

[Gavin Shan <gwshan@linux.vnet.ibm.com>]

On Mon, Jan 23, 2017 at 08:08:20PM +1100, Michael Ellerman wrote:
>Gavin Shan <gwshan@linux.vnet.ibm.com> writes:
>
>> This removes the unnecessary nested if statements in function
>> rtas_initialize(), to simplify the code. No functional changes
>> introduced.
>>
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/kernel/rtas.c | 33 ++++++++++++++++-----------------
>>  1 file changed, 16 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
>> index 112cc3b..9ba0f67 100644
>> --- a/arch/powerpc/kernel/rtas.c
>> +++ b/arch/powerpc/kernel/rtas.c
>> @@ -1145,31 +1145,30 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
>>  void __init rtas_initialize(void)
>>  {
>>  	unsigned long rtas_region = RTAS_INSTANTIATE_MAX;
>> +	const __be32 *basep, *entryp, *sizep;
>>  
>>  	/* Get RTAS dev node and fill up our "rtas" structure with infos
>>  	 * about it.
>>  	 */
>>  	rtas.dev = of_find_node_by_name(NULL, "rtas");
>> -	if (rtas.dev) {
>> -		const __be32 *basep, *entryp, *sizep;
>> -
>> -		basep = of_get_property(rtas.dev, "linux,rtas-base", NULL);
>> -		sizep = of_get_property(rtas.dev, "rtas-size", NULL);
>> -		if (basep != NULL && sizep != NULL) {
>			...
>> -		} else
>
>Previously we set rtas.dev to NULL if either basep or sizep was NULL.
>
>> -			rtas.dev = NULL;
>> -	}
>>  	if (!rtas.dev)
>>  		return;
>>  
>> +	basep = of_get_property(rtas.dev, "linux,rtas-base", NULL);
>> +	sizep = of_get_property(rtas.dev, "rtas-size", NULL);
>> +	if (basep == NULL && sizep == NULL) {
>
>But now you set it to NULL only if BOTH basep and sizep are NULL.
>
>Was that intentional? If so you need to mention it in the change log.
>
>> +		rtas.dev = NULL;
>> +		return;
>> +	}
>
>The proper negation of:
>
>	if (basep != NULL && sizep != NULL) {
>is:
>	if (basep == NULL || sizep == NULL) {
>

Thanks, Michael. It's not intentional. I'll update in v2.

Thanks,
Gavin