Re: [PATCH 3/4] error: Remove NULL checks on error_propagate() calls (again)

[Eric Blake]

On 7/23/20 8:38 AM, Markus Armbruster wrote:

> > > > +++ b/migration/colo.c
> > > > @@ -798,9 +798,7 @@ static void 
> > > > colo_incoming_process_checkpoint(MigrationIncomingState *mis,
> > > >          colo_send_message(mis->to_src_file,
> > > > COLO_MESSAGE_VMSTATE_LOADED,
> > > >                     &local_err);
> > > > -    if (local_err) {
> > > > -        error_propagate(errp, local_err);
> > > > -    }
> > > > +    error_propagate(errp, local_err);
> > > >    }
> > >
> > > As this is mechanical, it is fine. But there is now a further cleanup
> > > possible of passing errp directly to colo_send_message, and possibly
> > > dropping local_err altogether.
> >
> > True.
> >
> > The patch is small and simple enough for squashing in further manual
> > cleanups.  I'd like to first check whether a followup patch created with
> > the machinery I used for eliminating error_propagate() comes out better.
>
> Vladimir's scripts/coccinelle/errp-guard.cocci will take care of it.

Good to know.  Then I'm fine deferring those cleanups to the mechanical 
patches down the road, rather than a manual effort now.

> Eliminating error propagation altogether would be even better, but it's
> also more work: several void functions need to return bool instead.

Correct, but also doesn't change the fact that this patch is ready to go 
regardless of how much further cleanup we plan on doing.

> > > Reviewed-by: Eric Blake <eblake@redhat.com>
> >
> > Thanks!

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org