[Qemu-devel] [PATCH] target-alpha: fix subl and s8subl indentation

[Qemu-devel] [PATCH] target-alpha: fix subl and s8subl indentation

[Paolo Bonzini]

Two missing braces, one close and one open, fabulously let the code
compile.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-alpha/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index a9ef1a7..e7e319b 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -1927,6 +1927,7 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn)
                     else {
                         tcg_gen_neg_i64(cpu_ir[rc], cpu_ir[rb]);
                         tcg_gen_ext32s_i64(cpu_ir[rc], cpu_ir[rc]);
+                    }
                 }
             }
             break;
@@ -1991,7 +1992,7 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn)
                 } else {
                     if (islit)
                         tcg_gen_movi_i64(cpu_ir[rc], -lit);
-                    else
+                    else {
                         tcg_gen_neg_i64(cpu_ir[rc], cpu_ir[rb]);
                         tcg_gen_ext32s_i64(cpu_ir[rc], cpu_ir[rc]);
                     }
-- 
1.8.5.3

Re: [Qemu-devel] [PATCH] target-alpha: fix subl and s8subl indentation

[Richard Henderson]

On 03/15/2014 11:33 AM, Paolo Bonzini wrote:
> Two missing braces, one close and one open, fabulously let the code
> compile.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Wow.  That's ... amazing.

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH] target-alpha: fix subl and s8subl indentation

[Peter Maydell]

On 15 March 2014 18:33, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Two missing braces, one close and one open, fabulously let the code
> compile.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  target-alpha/translate.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target-alpha/translate.c b/target-alpha/translate.c
> index a9ef1a7..e7e319b 100644
> --- a/target-alpha/translate.c
> +++ b/target-alpha/translate.c
> @@ -1927,6 +1927,7 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn)
>                      else {
>                          tcg_gen_neg_i64(cpu_ir[rc], cpu_ir[rb]);
>                          tcg_gen_ext32s_i64(cpu_ir[rc], cpu_ir[rc]);
> +                    }
>                  }
>              }
>              break;
> @@ -1991,7 +1992,7 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn)
>                  } else {
>                      if (islit)
>                          tcg_gen_movi_i64(cpu_ir[rc], -lit);
> -                    else
> +                    else {
>                          tcg_gen_neg_i64(cpu_ir[rc], cpu_ir[rb]);
>                          tcg_gen_ext32s_i64(cpu_ir[rc], cpu_ir[rc]);
>                      }

I'm surprised checkpatch didn't insist you added braces to the "if"
half of this if-else as well.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] target-alpha: fix subl and s8subl indentation

[Paolo Bonzini]

Il 16/03/2014 00:47, Peter Maydell ha scritto:
> On 15 March 2014 18:33, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Two missing braces, one close and one open, fabulously let the code
>> compile.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  target-alpha/translate.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/target-alpha/translate.c b/target-alpha/translate.c
>> index a9ef1a7..e7e319b 100644
>> --- a/target-alpha/translate.c
>> +++ b/target-alpha/translate.c
>> @@ -1927,6 +1927,7 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn)
>>                      else {
>>                          tcg_gen_neg_i64(cpu_ir[rc], cpu_ir[rb]);
>>                          tcg_gen_ext32s_i64(cpu_ir[rc], cpu_ir[rc]);
>> +                    }
>>                  }
>>              }
>>              break;
>> @@ -1991,7 +1992,7 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn)
>>                  } else {
>>                      if (islit)
>>                          tcg_gen_movi_i64(cpu_ir[rc], -lit);
>> -                    else
>> +                    else {
>>                          tcg_gen_neg_i64(cpu_ir[rc], cpu_ir[rb]);
>>                          tcg_gen_ext32s_i64(cpu_ir[rc], cpu_ir[rc]);
>>                      }
>
> I'm surprised checkpatch didn't insist you added braces to the "if"
> half of this if-else as well.

I must admit I ignored it.  It makes the patch harder to review, since 
the bug is explicitly about missing (unmatched) braces.  I can add the 
braces throughout the file, but I don't think it's a reason to obfuscate 
this particular patch.

Paolo

Re: [Qemu-devel] [PATCH] target-alpha: fix subl and s8subl indentation

[Stuart Brady]

On Sat, Mar 15, 2014 at 01:40:26PM -0700, Richard Henderson wrote:
> On 03/15/2014 11:33 AM, Paolo Bonzini wrote:
> > Two missing braces, one close and one open, fabulously let the code
> > compile.
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Wow.  That's ... amazing.
> 
> Reviewed-by: Richard Henderson <rth@twiddle.net>

The *really* amazing thing about this (at least as far as I can see) is
the missing braces didn't alter the semantics of the code, except that
with the patch we no longer generate an extra TCG op with no effect...
-- 
Cheers,
Stuart