Re: [PATCH v6 30/30] hw/timer/sh_timer: Remove use of hw_error

[Philippe Mathieu-Daudé]

On 10/29/21 23:02, BALATON Zoltan wrote:
> The hw_error function calls abort and is not meant to be used by
> devices. Use qemu_log_mask instead to log and ignore invalid accesses.
> Also fix format strings to allow dropping type casts of hwaddr and use
> __func__ instead of hard coding function name in the message which
> were wrong in two cases.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/timer/sh_timer.c | 40 +++++++++++++++++++++++++---------------
>  1 file changed, 25 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/timer/sh_timer.c b/hw/timer/sh_timer.c
> index a6445092e4..8a586f2c4a 100644
> --- a/hw/timer/sh_timer.c
> +++ b/hw/timer/sh_timer.c
> @@ -10,7 +10,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "exec/memory.h"
> -#include "hw/hw.h"
> +#include "qemu/log.h"
>  #include "hw/irq.h"
>  #include "hw/sh4/sh.h"
>  #include "hw/timer/tmu012.h"
> @@ -75,11 +75,10 @@ static uint32_t sh_timer_read(void *opaque, hwaddr offset)
>          if (s->feat & TIMER_FEAT_CAPT) {
>              return s->tcpr;
>          }
> -        /* fall through */
> -    default:
> -        hw_error("sh_timer_read: Bad offset %x\n", (int)offset);
> -        return 0;
>      }
> +    qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n",
> +                  __func__, offset);
> +    return 0;

Note, keeping the default case allow to refactor for single return,
so it is easier to add trace event.

Anyhow,
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>