From mboxrd@z Thu Jan 1 00:00:00 1970 Received: by 2002:adf:e804:0:0:0:0:0 with SMTP id o4csp5502041wrm; Tue, 8 Oct 2019 11:26:20 -0700 (PDT) X-Google-Smtp-Source: APXvYqz5PyK600kuFpMLvACFzZKwX4a7LN4AJj/5Rx+OInw4nJbvGwJ/xSzAMUmwJA3H8Svg+MTQ X-Received: by 2002:a37:a9c1:: with SMTP id s184mr30351997qke.360.1570559180141; Tue, 08 Oct 2019 11:26:20 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1570559180; cv=none; d=google.com; s=arc-20160816; b=0T/4TqyTPfs5j9/MKtXQcIG776N/zb0g9b6cAwKy5fYJLtQD1Nm9CLDzQCBHN3EETZ ASoPPnege48K+WtvIf8wYHO4l7/ZFecj4qMIj6LjDYP3lBNSxtQOw2NsSORPb9vNHVjz LU/SpoSc6meGmJ5bHPJGzzNXAMKQ9S0CiGutP9Xgwb3oAFwuzQGKK5jCKd1hQPqmaSeY iibnDwskgM6f0Vh0Dss22GX1cr6UrkYE6+HwOh04IsNWbeq9ZI+KCxnfkW3/AMxeggg3 A/elQuH4+jUFIKxdFqz+t5SCjuXeo2AzYtyARql6NnaFerBzzRdLcSeXOvK20shqapG+ pLJw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:mime-version:user-agent :message-id:in-reply-to:date:references:subject:to:from; bh=Udq3hdfUoMlH38wVeKm45XrfjuR+5vTycRsqGuEaazk=; b=aos4lbVWjCGi/MySRhyzdsc2dS+tHV00U7iiax3Q5ytaBWC/LoYyBGdnc3APQKEvxz MY3oN94H+pc4kXVnre3zB+C0EpiGHZlMpsrR4HaPJBR58OCFmXKAompBiN1OGhYqV9NA FQpCz/a1MMyPvQTqr8X8KfzGaPBhz2x+gjS3RS/twaXjE1cbi6iKzbjGoRJbn1o0oBGD TyQZ8fwWfmt7tmT7Muc3Ch46Zjcf3GquUsd+cmPcR3oYRfsbtUTRiakta6M/wZxec1mI dkkkC7yA9kBnRITiOve556hHQWM50L6n6Js8fHKt+rA0gfY2drT3IcskBsI9yB6FGvjF 4PFw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org. [209.51.188.17]) by mx.google.com with ESMTPS id j30si16619869qki.47.2019.10.08.11.26.19 for (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 08 Oct 2019 11:26:20 -0700 (PDT) Received-SPF: pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; Authentication-Results: mx.google.com; spf=pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from localhost ([::1]:60808 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iHuBe-0002Zt-PB for alex.bennee@linaro.org; Tue, 08 Oct 2019 14:26:18 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:35786) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iHuAQ-0002YG-AK for qemu-arm@nongnu.org; Tue, 08 Oct 2019 14:25:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iHuAN-0002Vm-F2 for qemu-arm@nongnu.org; Tue, 08 Oct 2019 14:25:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44952) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1iHuAN-0002Sy-4j; Tue, 08 Oct 2019 14:24:59 -0400 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 00E77806A7C; Tue, 8 Oct 2019 18:24:54 +0000 (UTC) Received: from blackfin.pond.sub.org (unknown [10.36.118.123]) by smtp.corp.redhat.com (Postfix) with ESMTPS id BBF291001B32; Tue, 8 Oct 2019 18:24:35 +0000 (UTC) Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id F3DE31138648; Tue, 8 Oct 2019 20:24:31 +0200 (CEST) From: Markus Armbruster To: Greg Kurz Subject: Re: [PATCH v4 04/31] error: auto propagated local_err References: <20191001155319.8066-1-vsementsov@virtuozzo.com> <20191001155319.8066-5-vsementsov@virtuozzo.com> <87k19fgr7i.fsf@dusky.pond.sub.org> <20191008181921.288e69e4@bahia.lan> Date: Tue, 08 Oct 2019 20:24:31 +0200 In-Reply-To: <20191008181921.288e69e4@bahia.lan> (Greg Kurz's message of "Tue, 8 Oct 2019 18:19:21 +0200") Message-ID: <87y2xvccyo.fsf@dusky.pond.sub.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.6.2 (mx1.redhat.com [10.5.110.67]); Tue, 08 Oct 2019 18:24:54 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 X-BeenThere: qemu-arm@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: fam@euphon.net, pburton@wavecomp.com, peter.maydell@linaro.org, codyprime@gmail.com, jasowang@redhat.com, mark.cave-ayland@ilande.co.uk, qemu-devel@nongnu.org, Markus Armbruster , kraxel@redhat.com, sundeep.lkml@gmail.com, qemu-block@nongnu.org, quintela@redhat.com, arikalo@wavecomp.com, mst@redhat.com, mdroth@linux.vnet.ibm.com, pasic@linux.ibm.com, borntraeger@de.ibm.com, joel@jms.id.au, marcandre.lureau@redhat.com, rth@twiddle.net, farman@linux.ibm.com, ehabkost@redhat.com, sw@weilnetz.de, dgilbert@redhat.com, yuval.shaia@oracle.com, alex.williamson@redhat.com, Vladimir Sementsov-Ogievskiy , clg@kaod.org, stefanha@redhat.com, david@redhat.com, jsnow@redhat.com, david@gibson.dropbear.id.au, kwolf@redhat.com, integration@gluster.org, berrange@redhat.com, andrew@aj.id.au, cohuck@redhat.com, qemu-s390x@nongnu.org, mreitz@redhat.com, qemu-arm@nongnu.org, qemu-ppc@nongnu.org, pbonzini@redhat.com Errors-To: qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org Sender: "Qemu-arm" X-TUID: yQ/28Em/dPPi Greg Kurz writes: > On Tue, 08 Oct 2019 18:03:13 +0200 > Markus Armbruster wrote: > >> Vladimir Sementsov-Ogievskiy writes: >> >> > Here is introduced ERRP_AUTO_PROPAGATE macro, to be used at start of >> > functions with errp OUT parameter. >> > >> > It has three goals: >> > >> > 1. Fix issue with error_fatal & error_prepend/error_append_hint: user >> > can't see this additional information, because exit() happens in >> > error_setg earlier than information is added. [Reported by Greg Kurz] >> > >> > 2. Fix issue with error_abort & error_propagate: when we wrap >> > error_abort by local_err+error_propagate, resulting coredump will >> > refer to error_propagate and not to the place where error happened. >> > (the macro itself doesn't fix the issue, but it allows to [3.] drop all >> > local_err+error_propagate pattern, which will definitely fix the issue) >> > [Reported by Kevin Wolf] >> > >> > 3. Drop local_err+error_propagate pattern, which is used to workaround >> > void functions with errp parameter, when caller wants to know resulting >> > status. (Note: actually these functions could be merely updated to >> > return int error code). >> >> Starting with stating your goals is an excellent idea. But I'd love to >> next read a high-level description of how your patch achieves or enables >> achieving these goals. >> >> > Signed-off-by: Vladimir Sementsov-Ogievskiy >> > --- >> [...] >> > diff --git a/include/qapi/error.h b/include/qapi/error.h >> > index 9376f59c35..02f967ac1d 100644 >> > --- a/include/qapi/error.h >> > +++ b/include/qapi/error.h >> > @@ -322,6 +322,43 @@ void error_set_internal(Error **errp, >> > ErrorClass err_class, const char *fmt, ...) >> > GCC_FMT_ATTR(6, 7); >> > >> > +typedef struct ErrorPropagator { >> > + Error *local_err; >> > + Error **errp; >> > +} ErrorPropagator; >> > + >> > +static inline void error_propagator_cleanup(ErrorPropagator *prop) >> > +{ >> > + error_propagate(prop->errp, prop->local_err); >> > +} >> > + >> > +G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(ErrorPropagator, error_propagator_cleanup); >> > + >> > +/* >> > + * ERRP_AUTO_PROPAGATE >> > + * >> > + * This macro is created to be the first line of a function with Error **errp >> > + * OUT parameter. It's needed only in cases where we want to use error_prepend, >> > + * error_append_hint or dereference *errp. It's still safe (but useless) in >> > + * other cases. >> > + * >> > + * If errp is NULL or points to error_fatal, it is rewritten to point to a >> > + * local Error object, which will be automatically propagated to the original >> > + * errp on function exit (see error_propagator_cleanup). >> > + * >> > + * After invocation of this macro it is always safe to dereference errp >> > + * (as it's not NULL anymore) and to append hints (by error_append_hint) >> > + * (as, if it was error_fatal, we swapped it with a local_error to be >> > + * propagated on cleanup). >> >> Well, appending hints was always safe, it just didn't work with >> &error_fatal. Don't worry about that now, I'll probably want to polish >> this contract comment a bit anyway, but later. >> > > FWIW I've already posted this: > > Author: Greg Kurz > Date: Mon Oct 7 15:45:46 2019 +0200 > > error: Update error_append_hint()'s documenation > > error_setg() and error_propagate(), as well as their variants, cause > QEMU to terminate when called with &error_fatal or &error_abort. This > prevents to add hints since error_append_hint() isn't even called in > this case. > > It means that error_append_hint() should only be used with a local > error object, and then propagate this local error to the caller. > > Document this in . > > Signed-off-by: Greg Kurz > > Message-id: <156871563702.196432.5964411202152101367.stgit@bahia.lan> > https://patchwork.ozlabs.org/patch/1163278/ I marked your series containing this patch for review. I decided to review this one first. I clearly need look at yours as well before I can advise on how to proceed. [...]