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

2011-09-12 Thread 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 blauwir...@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 0x 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

2011-09-12 Thread Blue Swirl
On Mon, Sep 12, 2011 at 5:12 PM, Scott Wood scottw...@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 blauwir...@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 0x 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