On 9/11/20 7:38 AM, Christophe Leroy wrote:
> 
> 
> Le 10/09/2020 à 23:02, Cédric Le Goater a écrit :
>> ../arch/powerpc/lib/sstep.c: In function ‘mlsd_8lsd_ea’:
>> ../arch/powerpc/lib/sstep.c:225:3: error: suggest braces around empty body 
>> in an ‘if’ statement [-Werror=empty-body]
>>     ; /* Invalid form. Should already be checked for by caller! */
>>     ^
> 
> A small sentence explaining how this is fixed would be welcome, so that you 
> don't need to read the code the know what the commit does to fix the warning. 
> Also the subject should be more explicit.
> 
> 
> 
>>
>> Cc: Jordan Niethe <jniet...@gmail.com>
>> Signed-off-by: Cédric Le Goater <c...@kaod.org>
>> ---
>>   arch/powerpc/lib/sstep.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
>> index caee8cc77e19..14572af16e55 100644
>> --- a/arch/powerpc/lib/sstep.c
>> +++ b/arch/powerpc/lib/sstep.c
>> @@ -221,8 +221,9 @@ static nokprobe_inline unsigned long 
>> mlsd_8lsd_ea(unsigned int instr,
>>           ; /* Leave ea as is */
>>       else if (prefix_r && !ra)
>>           ea += regs->nip;
>> -    else if (prefix_r && ra)
>> +    else if (prefix_r && ra) {
>>           ; /* Invalid form. Should already be checked for by caller! */
>> +    }
> 
> You can't do that. Now checkpatch will complain that you don't have braces on 
> all legs of the if/else dance.

Should we fix checkpatch ?

> I think the last 'else if' should simply be removed entirely as it does 
> nothing. Eventually, just leave the comment, something like:
> 
>     /* (prefix_r && ra) is Invalid form. Should already be checked for by 
> caller! */
> 
> And if (prefix_r && ra) is not possible, then the previous if should just be 
> 'if (prefx_r)'

Indeed. I will rework that one.

Thanks,

C. 


> Christophe
> 
> 
>>         return ea;
>>   }
>>

Reply via email to