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

[zhanghailiang]

On 2014/9/17 0:24, Markus Armbruster wrote:
> 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.

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.

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