Re: [Qemu-devel] [PATCHv3 8/9] pseries: Clean up error reporting in ppc_spapr_init()

[Thomas Huth <thuth@redhat.com>]

On 18.01.2016 05:24, David Gibson wrote:
> This function includes a number of explicit fprintf()s for errors.
> Change these to use error_report() instead.
> 
> Also replace the single exit(EXIT_FAILURE) with an explicit exit(1), since
> the latter is the more usual idiom in qemu by a large margin.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 148ca5a..58f26cd 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1789,8 +1789,8 @@ static void ppc_spapr_init(MachineState *machine)
>      }
>  
>      if (spapr->rma_size > node0_size) {
> -        fprintf(stderr, "Error: Numa node 0 has to span the RMA (%#08"HWADDR_PRIx")\n",
> -                spapr->rma_size);
> +        error_report("Numa node 0 has to span the RMA (%#08"HWADDR_PRIx")",
> +                     spapr->rma_size);
>          exit(1);
>      }
>  
> @@ -1856,10 +1856,10 @@ static void ppc_spapr_init(MachineState *machine)
>          ram_addr_t hotplug_mem_size = machine->maxram_size - machine->ram_size;
>  
>          if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) {
> -            error_report("Specified number of memory slots %" PRIu64
> -                         " exceeds max supported %d",
> -                         machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
> -            exit(EXIT_FAILURE);
> +            error_report("Specified number of memory slots %"
> +                         PRIu64" exceeds max supported %d",
> +                machine->ram_slots, SPAPR_MAX_RAM_SLOTS);

Why did you change the indentation of the "machine->ram_slots, ..." line
here? The original looked better to me.

> +            exit(1);

EXIT_FAILURE still seems to be used quite often in the QEMU sources...
All in all, this hunk does not really change anything from a functional
point of view, so I'd like to suggest to omit this hunk completely
instead to avoid code churn here.

 Thomas