Re: [Qemu-devel] [PATCH v7 2/2] dump: Turn some functions to void to make code cleaner

[Eric Blake]

On 09/30/2014 03:20 AM, zhanghailiang wrote:
> 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 local_err.
> 
> Signed-off-by: zhanghailiang <address@hidden>
> ---
>  dump.c | 313 
> ++++++++++++++++++++++++++++++-----------------------------------
>  1 file changed, 143 insertions(+), 170 deletions(-)
> 


> @@ -348,49 +326,45 @@ static int write_elf_section(DumpState *s, int type, 
> Error **errp)
>      ret = fd_write_vmcore(&shdr, shdr_size, s);
>      if (ret < 0) {
>          dump_error(s, "dump: failed to write section header table", errp);
> -        return -1;
> +        return;
>      }
> -
> -    return 0;
>  }

The 'return' here is not in a loop, and therefore not necessary.

>  
> -static int write_data(DumpState *s, void *buf, int length, Error **errp)
> +static void write_data(DumpState *s, void *buf, int length, Error **errp)
>  {
>      int ret;
>  
>      ret = fd_write_vmcore(buf, length, s);
>      if (ret < 0) {
>          dump_error(s, "dump: failed to save memory", errp);
> -        return -1;
> +        return;
>      }
> -
> -    return 0;
>  }

and again.

>  
>  /* write the memroy to vmcore. 1 page per I/O. */

Please s/memroy/memory/ while touching this :)


> @@ -1706,7 +1680,6 @@ void qmp_dump_guest_memory(bool paging, const char 
> *file, bool has_begin,
>      } else {
>          create_vmcore(s, errp);
>      }
> -
>      g_free(s);
>  }

Looks a bit like a spurious line deletion in this hunk.

Findings are minor, so I'm fine if you add:
Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature