From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38679) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VobL6-0001Wy-R8 for qemu-devel@nongnu.org; Thu, 05 Dec 2013 10:59:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VobL1-0001dL-M3 for qemu-devel@nongnu.org; Thu, 05 Dec 2013 10:59:44 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55565) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VobL1-0001dA-C6 for qemu-devel@nongnu.org; Thu, 05 Dec 2013 10:59:39 -0500 Message-ID: <52A0A2E4.7060903@redhat.com> Date: Thu, 05 Dec 2013 16:59:32 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <87a9ginu92.fsf@blackfin.pond.sub.org> <529DD58C.8020408@redhat.com> <87y5423ut9.fsf@blackfin.pond.sub.org> <20131203213348.3f4e345e@thinkpad> <52A05767.60703@redhat.com> <20131205163228.4700f3cc@nial.usersys.redhat.com> In-Reply-To: <20131205163228.4700f3cc@nial.usersys.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH v1 0/5] Add error_abort and associated cleanups List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: Peter Crosthwaite , Markus Armbruster , afaerber@suse.de, qemu-devel@nongnu.org Il 05/12/2013 16:32, Igor Mammedov ha scritto: >>> > > * make Error* mandatory for all void functions > one more advantage: > + not need to pepper every property setter/getter with local_error + error_propagate(), > i.e. reduced code duplication. Note that for something like tail calls there is no need for error_propagate. See get_enum/set_enum (and a lot more examples) in hw/core/qdev-properties.c. With your proposal, this: visit_type_bool(v, &value, name, &local_err); if (local_err) { error_propagate(errp, local_err); return; } bit_prop_set(dev, prop, value); could indeed become visit_type_bool(v, &value, name, errp); bit_prop_set(dev, prop, value); because both caller and callee return void. This is because both the caller (prop_set_bit) and callee (visit_type_bool) are void. But here come the next point: >>> > > + consistent >>> > > + almost predictable (because in C you can ignore return values) > there is no return values from void functions. You can ignore return values from int-returning functions. So if I see func(..., NULL); func(..., errp); it's not clear if it aborts on error, or just eats the error. > > - requires manual effort to abide to the policy > with assert inside API there is no manual effort. But as Marcus > noted these errors will be only runtime detectable :( I referred to manual effort of adding assertions in leaf functions. Just one missing assertion can be very problematic if non-leafs start relying on this behavior. I think Error is hard to misuse if you don't mind being verbose. This proposal would cut the verbosity (not sure how much, but it definitely could) but it would be easier to misuse. In terms of Rusty's API design manifesto (http://sweng.the-davies.net/Home/rustys-api-design-manifesto) your proposal would move the API from 4 to 3 or 2: 4. Follow common convention and you'll get it right. 3. Read the documentation and you'll get it right. 2. Read the implementation and you'll get it right. Paolo