[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 2/2] dump: Don't return error code when retur
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v5 2/2] dump: Don't return error code when return an Error object |
Date: |
Tue, 16 Sep 2014 18:24:44 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
zhanghailiang <address@hidden> 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 <address@hidden>
> ---
> 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.
> - 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.
> }
> }
[...]