Patchwork [2/3] spapr: Fix compiler warning for some versions of gcc (h_remove)

login
register
mail settings
Submitter Stefan Weil
Date June 24, 2013, 5:48 p.m.
Message ID <1372096130-24994-3-git-send-email-sw@weilnetz.de>
Download mbox | patch
Permalink /patch/253939/
State Superseded
Headers show

Comments

Stefan Weil - June 24, 2013, 5:48 p.m.
i686-w64-mingw32-gcc (GCC) 4.6.3 from Debian wheezy reports this warning:

hw/ppc/spapr_hcall.c:188:1: warning:
 control reaches end of non-void function [-Wreturn-type]

Replacing the 4th case REMOVE_HW (which is currently unused) by the default
case fixes this warning.

The assertion is dead code because all possible cases are handled in the
switch statements, so remove it. This avoids future warnings from static
code analyzers.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 hw/ppc/spapr_hcall.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
Paolo Bonzini - June 24, 2013, 8:27 p.m.
Il 24/06/2013 19:48, Stefan Weil ha scritto:
> i686-w64-mingw32-gcc (GCC) 4.6.3 from Debian wheezy reports this warning:
> 
> hw/ppc/spapr_hcall.c:188:1: warning:
>  control reaches end of non-void function [-Wreturn-type]
> 
> Replacing the 4th case REMOVE_HW (which is currently unused) by the default
> case fixes this warning.
> 
> The assertion is dead code because all possible cases are handled in the
> switch statements, so remove it. This avoids future warnings from static
> code analyzers.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  hw/ppc/spapr_hcall.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 00f21f5..d49ce53 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -180,11 +180,9 @@ static target_ulong h_remove(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>      case REMOVE_PARM:
>          return H_PARAMETER;
>  
> -    case REMOVE_HW:
> +    default: /* REMOVE_HW */
>          return H_HARDWARE;
>      }
> -
> -    assert(0);
>  }
>  
>  #define H_BULK_REMOVE_TYPE             0xc000000000000000ULL
> 

Does s/assert(0)/abort()/ fix the warning too?  That's clearer.

Paolo
Stefan Weil - June 24, 2013, 8:35 p.m.
Am 24.06.2013 22:27, schrieb Paolo Bonzini:
> Il 24/06/2013 19:48, Stefan Weil ha scritto:
>> i686-w64-mingw32-gcc (GCC) 4.6.3 from Debian wheezy reports this warning:
>>
>> hw/ppc/spapr_hcall.c:188:1: warning:
>>  control reaches end of non-void function [-Wreturn-type]
>>
>> Replacing the 4th case REMOVE_HW (which is currently unused) by the default
>> case fixes this warning.
>>
>> The assertion is dead code because all possible cases are handled in the
>> switch statements, so remove it. This avoids future warnings from static
>> code analyzers.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>  hw/ppc/spapr_hcall.c |    4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 00f21f5..d49ce53 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -180,11 +180,9 @@ static target_ulong h_remove(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>>      case REMOVE_PARM:
>>          return H_PARAMETER;
>>  
>> -    case REMOVE_HW:
>> +    default: /* REMOVE_HW */
>>          return H_HARDWARE;
>>      }
>> -
>> -    assert(0);
>>  }
>>  
>>  #define H_BULK_REMOVE_TYPE             0xc000000000000000ULL
>>
> Does s/assert(0)/abort()/ fix the warning too?  That's clearer.
>
> Paolo

Yes, that also fixes this warning. A better compiler or a static
code checker might then complain about dead code because all
possible cases are handled in the switch statement.

Stefan
Paolo Bonzini - June 24, 2013, 9:07 p.m.
Il 24/06/2013 22:35, Stefan Weil ha scritto:
> Am 24.06.2013 22:27, schrieb Paolo Bonzini:
>> Il 24/06/2013 19:48, Stefan Weil ha scritto:
>>> i686-w64-mingw32-gcc (GCC) 4.6.3 from Debian wheezy reports this warning:
>>>
>>> hw/ppc/spapr_hcall.c:188:1: warning:
>>>  control reaches end of non-void function [-Wreturn-type]
>>>
>>> Replacing the 4th case REMOVE_HW (which is currently unused) by the default
>>> case fixes this warning.
>>>
>>> The assertion is dead code because all possible cases are handled in the
>>> switch statements, so remove it. This avoids future warnings from static
>>> code analyzers.
>>>
>>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>>> ---
>>>  hw/ppc/spapr_hcall.c |    4 +---
>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>>> index 00f21f5..d49ce53 100644
>>> --- a/hw/ppc/spapr_hcall.c
>>> +++ b/hw/ppc/spapr_hcall.c
>>> @@ -180,11 +180,9 @@ static target_ulong h_remove(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>>>      case REMOVE_PARM:
>>>          return H_PARAMETER;
>>>  
>>> -    case REMOVE_HW:
>>> +    default: /* REMOVE_HW */
>>>          return H_HARDWARE;
>>>      }
>>> -
>>> -    assert(0);
>>>  }
>>>  
>>>  #define H_BULK_REMOVE_TYPE             0xc000000000000000ULL
>>>
>> Does s/assert(0)/abort()/ fix the warning too?  That's clearer.
> 
> Yes, that also fixes this warning. A better compiler or a static
> code checker might then complain about dead code because all
> possible cases are handled in the switch statement.

We'll get there when we see it... and such a compiler can well learn to
ignore a dead abort().

Paolo

Patch

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 00f21f5..d49ce53 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -180,11 +180,9 @@  static target_ulong h_remove(PowerPCCPU *cpu, sPAPREnvironment *spapr,
     case REMOVE_PARM:
         return H_PARAMETER;
 
-    case REMOVE_HW:
+    default: /* REMOVE_HW */
         return H_HARDWARE;
     }
-
-    assert(0);
 }
 
 #define H_BULK_REMOVE_TYPE             0xc000000000000000ULL