qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCHv3 8/9] pseries: Clean up error reporting in ppc_spapr_init()
Date: Mon, 18 Jan 2016 11:06:03 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Thomas Huth <address@hidden> writes:

> 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 <address@hidden>
>> ---
>>  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.

Agreed.

>> +            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.

It makes the code locally consistent, so I'd keep it.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]