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

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

[Blue Swirl]

[-- Attachment #1: Type: text/plain, Size: 679 bytes --]

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 <blauwirbel@gmail.com>
---
 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;
-- 
1.6.2.4

[-- Attachment #2: 0007-openpic-avoid-a-warning-from-clang-analyzer.patch --]
[-- Type: text/x-diff, Size: 1169 bytes --]

From 3c4f48ac6edafd3fae07d081e66b40e8144b1818 Mon Sep 17 00:00:00 2001
Message-Id: <3c4f48ac6edafd3fae07d081e66b40e8144b1818.1315150286.git.blauwirbel@gmail.com>
In-Reply-To: <70f99a25b7732d4c9ea54f74c089ccb9bb323ea6.1315150286.git.blauwirbel@gmail.com>
References: <70f99a25b7732d4c9ea54f74c089ccb9bb323ea6.1315150286.git.blauwirbel@gmail.com>
From: Blue Swirl <blauwirbel@gmail.com>
Date: Sun, 4 Sep 2011 11:19:33 +0000
Subject: [PATCH 7/9] openpic: avoid a warning from clang analyzer

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 <blauwirbel@gmail.com>
---
 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;
-- 
1.7.2.5

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

[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<blauwirbel@gmail.com>
> ---
>   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

[Blue Swirl]

On Mon, Sep 5, 2011 at 6:48 AM, Paolo Bonzini <pbonzini@redhat.com> 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<blauwirbel@gmail.com>
>> ---
>>  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

[Alexander Graf]

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

> On Mon, Sep 5, 2011 at 6:48 AM, Paolo Bonzini <pbonzini@redhat.com> 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<blauwirbel@gmail.com>
>>> ---
>>>  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

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

[Scott Wood]

On 09/04/2011 10:52 AM, 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 <blauwirbel@gmail.com>
> ---
>  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;

What's special about IDE?  Shouldn't it return 0xffffffff as some other
functions (e.g. openpic_gbl_read) do with unrecognized registers?  Then
there's openpic_src_read() which has still different behavior for the
same registers. :-P

Note that this function is only ever called with a constant in "reg".
Since it's a static function and all call sites could have been
verified, this could be considered a flaw in clang's analyzer.  This
workaround will prevent GCC from issuing a warning if a new caller is
added that passes a different constant value.

The best answer is probably to just get rid of this function and have
the caller refer to opp->src[n_irq].whatever directly.  write_IRQreg()
could be split into something like set_src_ipvp() and set_src_ide().

-Scott

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

[Blue Swirl]

On Mon, Sep 12, 2011 at 5:12 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 09/04/2011 10:52 AM, 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 <blauwirbel@gmail.com>
>> ---
>>  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;
>
> What's special about IDE?  Shouldn't it return 0xffffffff as some other
> functions (e.g. openpic_gbl_read) do with unrecognized registers?  Then
> there's openpic_src_read() which has still different behavior for the
> same registers. :-P
>
> Note that this function is only ever called with a constant in "reg".
> Since it's a static function and all call sites could have been
> verified, this could be considered a flaw in clang's analyzer.  This
> workaround will prevent GCC from issuing a warning if a new caller is
> added that passes a different constant value.
>
> The best answer is probably to just get rid of this function and have
> the caller refer to opp->src[n_irq].whatever directly.  write_IRQreg()
> could be split into something like set_src_ipvp() and set_src_ide().

Alex posted patches to that effect:
http://lists.nongnu.org/archive/html/qemu-devel/2011-09/msg00947.html
http://lists.nongnu.org/archive/html/qemu-devel/2011-09/msg00948.html