Re: [Qemu-devel] [PATCH v5 2/2] dump: Don't return error code when return an Error object

[zhanghailiang <zhang.zhanghailiang@huawei.com>]

On 2014/9/17 0:24, Markus Armbruster wrote:
> zhanghailiang <zhang.zhanghailiang@huawei.com> writes:
>
>> Functions shouldn't return an error code and an Error object at the same time.
>> Turn all these functions that returning Error object to void.
>> We also judge if a function success or fail by reference to the *errp.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> ---
>>   dump.c | 244 +++++++++++++++++++++++++----------------------------------------
>>   1 file changed, 94 insertions(+), 150 deletions(-)
>>
>> diff --git a/dump.c b/dump.c
>> index 07d2300..b2ed846 100644
>> --- a/dump.c
>> +++ b/dump.c
> [...]
>>   /* write the memroy to vmcore. 1 page per I/O. */
>> -static int write_memory(DumpState *s, GuestPhysBlock *block, ram_addr_t start,
>> +static void write_memory(DumpState *s, GuestPhysBlock *block, ram_addr_t start,
>>                           int64_t size, Error **errp)
>>   {
>>       int64_t i;
>> -    int ret;
>>
>>       for (i = 0; i < size / TARGET_PAGE_SIZE; i++) {
>> -        ret = write_data(s, block->host_addr + start + i * TARGET_PAGE_SIZE,
>> +        write_data(s, block->host_addr + start + i * TARGET_PAGE_SIZE,
>>                            TARGET_PAGE_SIZE, errp);
>
> Please fix up indentation of arguments.
>

OK.

>> -        if (ret < 0) {
>> -            return ret;
>> +        if (*errp) {
>> +            return;
>
> This breaks when a caller choses to ignore errors by passing a null
> argument for errp.
>
> For static functions that may be okay.  But in general, we support null
> arguments by coding like this:
>
>      Error *local_err = NULL;
>      int64_t i;
>
>      for (i = 0; i < size / TARGET_PAGE_SIZE; i++) {
>          write_data(s, block->host_addr + start + i * TARGET_PAGE_SIZE,
>                     TARGET_PAGE_SIZE, &local_err);
>          if (local_err) {
>              error_propagate(errp, local_err);
>              return;
>          }
>      }
>
> I feel it's better to stick to this convention.
>
> More of the same elsewhere.
>

OK, will do it, Thanks.

>>           }
>>       }
> [...]
>
> .
>