On Tue, 07/07 15:35, Stefan Hajnoczi wrote:
> On Tue, Jun 30, 2015 at 09:19:44PM +0800, Fam Zheng wrote:
> > diff --git a/async.c b/async.c
> > index 06971f4..1d70cfd 100644
> > --- a/async.c
> > +++ b/async.c
> > @@ -290,12 +290,17 @@ AioContext *aio_context_new(Error **errp)
> > {
> > int ret;
> > AioContext *ctx;
> > + Error *local_err = NULL;
> > +
> > ctx = (AioContext *) g_source_new(&aio_source_funcs,
> > sizeof(AioContext));
> > + aio_context_setup(ctx, &local_err);
> > + if (local_err) {
> > + error_propagate(errp, local_err);
>
> Is there any reason to introduce local_err? errp can be passed directly
> into aio_context_setup().
It's used for catching failure of aio_context_setup, because the convention is
errp can be NULL.
>
> > + goto fail;
> > + }
> > ret = event_notifier_init(&ctx->notifier, false);
> > if (ret < 0) {
> > - g_source_destroy(&ctx->source);
> > - error_setg_errno(errp, -ret, "Failed to initialize event
> > notifier");
> > - return NULL;
> > + goto fail;
> > }
> > g_source_set_can_recurse(&ctx->source, true);
> > aio_set_event_notifier(ctx, &ctx->notifier,
> > @@ -307,6 +312,10 @@ AioContext *aio_context_new(Error **errp)
> > timerlistgroup_init(&ctx->tlg, aio_timerlist_notify, ctx);
> >
> > return ctx;
> > +fail:
> > + g_source_destroy(&ctx->source);
> > + error_setg_errno(errp, -ret, "Failed to initialize event notifier");
>
> If aio_context_setup() failed then this hits the *errp == NULL assertion
> failure in error_setg_errno(). This call shouldn't be moved away from
> the event_notifier_init() call.
Yes, will fix.
Fam
