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 <address@hidden> 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 <address@hidden>
>> ---
>>  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