From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.2 required=3.0 tests=FROM_EXCESS_BASE64, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C0C95C49ED7 for ; Thu, 19 Sep 2019 14:57:53 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 992F920882 for ; Thu, 19 Sep 2019 14:57:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 992F920882 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:45288 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iAxsW-000866-QQ for qemu-devel@archiver.kernel.org; Thu, 19 Sep 2019 10:57:52 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:59144) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iAxlX-0001cP-7M for qemu-devel@nongnu.org; Thu, 19 Sep 2019 10:50:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iAxlU-0002TT-7g for qemu-devel@nongnu.org; Thu, 19 Sep 2019 10:50:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42442) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1iAxlL-0002NK-NN; Thu, 19 Sep 2019 10:50:27 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 842C42A09A0; Thu, 19 Sep 2019 14:50:25 +0000 (UTC) Received: from redhat.com (ovpn-112-51.ams2.redhat.com [10.36.112.51]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A0C3D60872; Thu, 19 Sep 2019 14:49:50 +0000 (UTC) Date: Thu, 19 Sep 2019 15:49:48 +0100 From: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= To: Eric Blake Subject: Re: [RFC] error: auto propagated local_err Message-ID: <20190919144948.GR20217@redhat.com> References: <20190918130244.24257-1-vsementsov@virtuozzo.com> <20190919091720.GB10163@localhost.localdomain> <57483252-273c-4606-47a8-eddeb840109a@redhat.com> <35c972e1-bdb5-cbcb-ed45-6a51f19af98c@virtuozzo.com> <696673be-95c8-3f75-551c-26fccd230eb1@virtuozzo.com> <152afb5b-8efb-d968-d595-94f58ad02a04@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <152afb5b-8efb-d968-d595-94f58ad02a04@redhat.com> User-Agent: Mutt/1.12.1 (2019-06-15) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Thu, 19 Sep 2019 14:50:25 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= Cc: "fam@euphon.net" , "peter.maydell@linaro.org" , "mst@redhat.com" , "codyprime@gmail.com" , "mark.cave-ayland@ilande.co.uk" , "qemu-devel@nongnu.org" , "armbru@redhat.com" , "kraxel@redhat.com" , "mreitz@redhat.com" , "qemu-block@nongnu.org" , "quintela@redhat.com" , "david@redhat.com" , "mdroth@linux.vnet.ibm.com" , "pasic@linux.ibm.com" , "borntraeger@de.ibm.com" , "marcandre.lureau@redhat.com" , "rth@twiddle.net" , "farman@linux.ibm.com" , "groug@kaod.org" , "dgilbert@redhat.com" , "alex.williamson@redhat.com" , "qemu-arm@nongnu.org" , "stefanha@redhat.com" , "jsnow@redhat.com" , "david@gibson.dropbear.id.au" , Kevin Wolf , Vladimir Sementsov-Ogievskiy , "cohuck@redhat.com" , "qemu-s390x@nongnu.org" , "sundeep.lkml@gmail.com" , "qemu-ppc@nongnu.org" , "pbonzini@redhat.com" Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On Thu, Sep 19, 2019 at 09:44:14AM -0500, Eric Blake wrote: > On 9/19/19 9:30 AM, Vladimir Sementsov-Ogievskiy wrote: > > >> > >> To the same topic, of minimization: should we always call MAKE_ERRP_SAFE at function top, or only > >> in block, where it is needed (assume, we dereference it only inside some "if" or "while"? > >> Kevin, is something bad in propagation, when it not related to error_abort? > >> > >> > > > > Or, even, we may use MAKE_ERRP_SAFE on every function, which have Error **errp argument, even if we neither > > dereference it nor append hints? Is it what you propose by "SINGLE paradigm"? It's of course simpler to script, > > to check in checkpatch and to maintain. > > Yes. The simpler we make the rules, and the less boilerplate it entails, > the more likely that we can: > > a) avoid exceptions and corner cases that cost review time > b) automate the conversion into complying with the rule > c) codify those rules in syntax check to ensure they are followed > post-conversion > > ALWAYS using MAKE_ERRP_SAFE() on entry to any function that has an Error > **errp parameter is dirt-simple to explain. It has no performance > penalty if the user passed in a normal error or error_abort (the cost of > an 'if' hidden in the macro is probably negligible compared to > everything else we do), and has no semantic penalty if the user passed > in NULL or error_fatal (we now get the behavior we want with less > boilerplate). > > Having to think 'does this method require me to use MAKE_ERRP_SAFE, or > can I omit it?' does not provide the same simplicity. The flipside is that MAKE_ERRP_SAFE hides a bunch of logic, so you don't really know what its doing without looking at it, and this is QEMU custom concept so one more thing to learn for new contributors. While I think it is a nice trick, personally I think we would be better off if we simply used a code pattern which does not require de-referencing 'errp' at all, aside from exceptional cases. IOW, no added macro in 95% of all our methods using Error **errp. There are lots of neat things we could do with auto-cleanup functions we I think we need to be wary of hiding too much cleverness behind macros when doing so overall. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|