Re: [Qemu-devel] [PATCH 7/9] openpic: avoid a warning from clang analyzer

2011-09-04 Thread Paolo Bonzini 

On 09/04/2011 05:52 PM, Blue Swirl wrote:

Avoid this warning by clang analyzer by defining a default case:
/src/qemu/hw/openpic.c:477:5: warning: Undefined or garbage value
returned to caller
 return retval;

Signed-off-by: Blue Swirl
---
  hw/openpic.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/openpic.c b/hw/openpic.c
index 26c96e2..4b883ac 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -469,6 +469,7 @@ static inline uint32_t read_IRQreg (openpic_t
*opp, int n_IRQ, uint32_t reg)
  case IRQ_IPVP:
  retval = opp->src[n_IRQ].ipvp;
  break;
+default:
  case IRQ_IDE:
  retval = opp->src[n_IRQ].ide;
  break;


Looks wrong, perhaps it should return 0?

Paolo

Re: [Qemu-devel] [PATCH 7/9] openpic: avoid a warning from clang analyzer

2011-09-05 Thread Blue Swirl 
On Mon, Sep 5, 2011 at 6:48 AM, Paolo Bonzini  wrote:
> On 09/04/2011 05:52 PM, Blue Swirl wrote:
>>
>> Avoid this warning by clang analyzer by defining a default case:
>> /src/qemu/hw/openpic.c:477:5: warning: Undefined or garbage value
>> returned to caller
>>     return retval;
>>
>> Signed-off-by: Blue Swirl
>> ---
>>  hw/openpic.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/openpic.c b/hw/openpic.c
>> index 26c96e2..4b883ac 100644
>> --- a/hw/openpic.c
>> +++ b/hw/openpic.c
>> @@ -469,6 +469,7 @@ static inline uint32_t read_IRQreg (openpic_t
>> *opp, int n_IRQ, uint32_t reg)
>>      case IRQ_IPVP:
>>          retval = opp->src[n_IRQ].ipvp;
>>          break;
>> +    default:
>>      case IRQ_IDE:
>>          retval = opp->src[n_IRQ].ide;
>>          break;
>
> Looks wrong, perhaps it should return 0?

The only possible values are IRQ_IDE and IRQ_IPVP.

The function is actually baroque, it's as easy to use
read_IRQreg(opp, IRQ_DBL0 + n_dbl, IRQ_IPVP);
as the shorter
opp->src[IRQ_DBL0 + n_dbl].ipvp;

The reason seems to be that write_IRQreg is more complex. I'd replace
both with {read,write}_{ide,ipvp} without the switch.

Re: [Qemu-devel] [PATCH 7/9] openpic: avoid a warning from clang analyzer

2011-09-07 Thread Alexander Graf 

On 05.09.2011, at 20:41, Blue Swirl wrote:

> On Mon, Sep 5, 2011 at 6:48 AM, Paolo Bonzini  wrote:
>> On 09/04/2011 05:52 PM, Blue Swirl wrote:
>>> 
>>> Avoid this warning by clang analyzer by defining a default case:
>>> /src/qemu/hw/openpic.c:477:5: warning: Undefined or garbage value
>>> returned to caller
>>> return retval;
>>> 
>>> Signed-off-by: Blue Swirl
>>> ---
>>>  hw/openpic.c |1 +
>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>> 
>>> diff --git a/hw/openpic.c b/hw/openpic.c
>>> index 26c96e2..4b883ac 100644
>>> --- a/hw/openpic.c
>>> +++ b/hw/openpic.c
>>> @@ -469,6 +469,7 @@ static inline uint32_t read_IRQreg (openpic_t
>>> *opp, int n_IRQ, uint32_t reg)
>>>  case IRQ_IPVP:
>>>  retval = opp->src[n_IRQ].ipvp;
>>>  break;
>>> +default:
>>>  case IRQ_IDE:
>>>  retval = opp->src[n_IRQ].ide;
>>>  break;
>> 
>> Looks wrong, perhaps it should return 0?
> 
> The only possible values are IRQ_IDE and IRQ_IPVP.
> 
> The function is actually baroque, it's as easy to use
> read_IRQreg(opp, IRQ_DBL0 + n_dbl, IRQ_IPVP);
> as the shorter
> opp->src[IRQ_DBL0 + n_dbl].ipvp;
> 
> The reason seems to be that write_IRQreg is more complex. I'd replace
> both with {read,write}_{ide,ipvp} without the switch.

I agree. Let me assemble a patch.


Alex