Re: [PATCH v2 09/10] hw/intc: Remove redundant statement in exynos4210_combiner_read()

[Peter Maydell]

On Tue, 25 Aug 2020 at 12:26, Chen Qun <kuhn.chenqun@huawei.com> wrote:
>
> Clang static code analyzer show warning:
> hw/intc/exynos4210_combiner.c:231:9: warning: Value stored to 'val' is never 
> read
>         val = s->reg_set[offset >> 2];
>
> The default value of 'val' is '0', so we can break the 'default' branch and 
> return 'val'.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
> ---
> Cc: Igor Mitsyanko <i.mitsyanko@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/intc/exynos4210_combiner.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/intc/exynos4210_combiner.c b/hw/intc/exynos4210_combiner.c
> index b8561e4180..e2e745bbaa 100644
> --- a/hw/intc/exynos4210_combiner.c
> +++ b/hw/intc/exynos4210_combiner.c
> @@ -228,8 +228,7 @@ exynos4210_combiner_read(void *opaque, hwaddr offset, 
> unsigned size)
>              hw_error("exynos4210.combiner: overflow of reg_set by 0x"
>                      TARGET_FMT_plx "offset\n", offset);
>          }
> -        val = s->reg_set[offset >> 2];
> -        return 0;
> +        break;
>      }
>      return val;
>  }

The code as it stands is definitely wrong, but I'm not sure
this is the correct fix. Surely the intention must have been
to return the actual register value from the reg_set[] array,
not to return 0 ?

I suspect the correct fix here is simply to delete the
"return 0" line and leave the assignment to val as it is.
Ideally you should check the h/w datasheet to confirm.

thanks
-- PMM