Re: [PATCH v2 01/55] hw/core: Make do_unaligned_access noreturn

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> While we may have had some thought of allowing system-mode
> to return from this hook, we have no guests that require this.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/hw/core/tcg-cpu-ops.h  | 3 ++-
>  target/alpha/cpu.h             | 4 ++--
>  target/arm/internals.h         | 3 ++-
>  target/microblaze/cpu.h        | 2 +-
>  target/mips/tcg/tcg-internal.h | 4 ++--
>  target/nios2/cpu.h             | 4 ++--
>  target/ppc/internal.h          | 4 ++--
>  target/riscv/cpu.h             | 2 +-
>  target/s390x/s390x-internal.h  | 4 ++--
>  target/sh4/cpu.h               | 4 ++--
>  target/xtensa/cpu.h            | 4 ++--
>  target/hppa/cpu.c              | 7 ++++---
>  12 files changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
> index eab27d0c03..ee0795def4 100644
> --- a/include/hw/core/tcg-cpu-ops.h
> +++ b/include/hw/core/tcg-cpu-ops.h
> @@ -72,10 +72,11 @@ struct TCGCPUOps {
>                                    MemTxResult response, uintptr_t retaddr);
>      /**
>       * @do_unaligned_access: Callback for unaligned access handling
> +     * The callback must exit via raising an exception.
>       */
>      void (*do_unaligned_access)(CPUState *cpu, vaddr addr,
>                                  MMUAccessType access_type,
> -                                int mmu_idx, uintptr_t retaddr);
> +                                int mmu_idx, uintptr_t retaddr) 
> QEMU_NORETURN;
>  
>      /**
>       * @adjust_watchpoint_address: hack for cpu_check_watchpoint used by ARM
> diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h
> index 82df108967..6eb3fcc63e 100644
> --- a/target/alpha/cpu.h
> +++ b/target/alpha/cpu.h
> @@ -283,8 +283,8 @@ hwaddr alpha_cpu_get_phys_page_debug(CPUState *cpu, vaddr 
> addr);
>  int alpha_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
>  int alpha_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>  void alpha_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
> -                                   MMUAccessType access_type,
> -                                   int mmu_idx, uintptr_t retaddr);
> +                                   MMUAccessType access_type, int mmu_idx,
> +                                   uintptr_t retaddr) QEMU_NORETURN;

These trailing QEMU_NORETURN's seem to be fairly uncommon in the
existing code. Indeed I'd glanced at this code and was about to suggest
one was added. IMHO is scans better when your reading the return type
for a function and you can always do:

  void QEMU_NORETURN
  foo_function(bar args);

if you are worried about over indentation. Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée