Re: [Qemu-devel] [PATCH 7/9] openpic: avoid a warning from clang analyzer
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
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
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