Re: [PATCH v2 1/3] hw/timer/slavio_timer: Remove useless check for NULL t->timer - Philippe Mathieu-Daudé

From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org
Cc: Richard Henderson <richard.henderson@linaro.org>,
	KONRAD Frederic <frederic.konrad@adacore.com>,
	Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>,
	Fabien Chouteau <chouteau@adacore.com>
Subject: Re: [PATCH v2 1/3] hw/timer/slavio_timer: Remove useless check for NULL t->timer
Date: Mon, 21 Oct 2019 16:03:32 +0200	[thread overview]
Message-ID: <17865d60-9d78-e876-90c3-f8f7363f689c@redhat.com> (raw)
In-Reply-To: <20191021134357.14266-2-peter.maydell@linaro.org>

On 10/21/19 3:43 PM, Peter Maydell wrote:
> In the slavio timer devcie, the ptimer TimerContext::timer is

typo "device"

> always created by slavio_timer_init(), so there's no need to
> check it for NULL; remove the single unneeded NULL check.
> 
> This will be useful to avoid compiler/Coverity errors when
> a subsequent change adds a use of t->timer before the location
> we currently do the NULL check.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   hw/timer/slavio_timer.c | 12 +++++-------
>   1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/timer/slavio_timer.c b/hw/timer/slavio_timer.c
> index 692d213897d..890dd53f8d8 100644
> --- a/hw/timer/slavio_timer.c
> +++ b/hw/timer/slavio_timer.c
> @@ -227,13 +227,11 @@ static void slavio_timer_mem_writel(void *opaque, hwaddr addr,
>               // set limit, reset counter
>               qemu_irq_lower(t->irq);
>               t->limit = val & TIMER_MAX_COUNT32;
> -            if (t->timer) {
> -                if (t->limit == 0) { /* free-run */
> -                    ptimer_set_limit(t->timer,
> -                                     LIMIT_TO_PERIODS(TIMER_MAX_COUNT32), 1);
> -                } else {
> -                    ptimer_set_limit(t->timer, LIMIT_TO_PERIODS(t->limit), 1);
> -                }
> +            if (t->limit == 0) { /* free-run */
> +                ptimer_set_limit(t->timer,
> +                                 LIMIT_TO_PERIODS(TIMER_MAX_COUNT32), 1);
> +            } else {
> +                ptimer_set_limit(t->timer, LIMIT_TO_PERIODS(t->limit), 1);
>               }
>           }
>           break;
>