From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MzsxL-0005lm-7Z for qemu-devel@nongnu.org; Mon, 19 Oct 2009 10:11:27 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MzsxG-0005gB-2o for qemu-devel@nongnu.org; Mon, 19 Oct 2009 10:11:26 -0400 Received: from [199.232.76.173] (port=33168 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MzsxF-0005g8-Sp for qemu-devel@nongnu.org; Mon, 19 Oct 2009 10:11:21 -0400 Received: from e4.ny.us.ibm.com ([32.97.182.144]:52398) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1MzsxF-0000IW-LR for qemu-devel@nongnu.org; Mon, 19 Oct 2009 10:11:21 -0400 Received: from d01relay03.pok.ibm.com (d01relay03.pok.ibm.com [9.56.227.235]) by e4.ny.us.ibm.com (8.14.3/8.13.1) with ESMTP id n9JE3cGH006098 for ; Mon, 19 Oct 2009 10:03:38 -0400 Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by d01relay03.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id n9JEBKeB060474 for ; Mon, 19 Oct 2009 10:11:20 -0400 Received: from d01av01.pok.ibm.com (d03av01 [127.0.0.1]) by d01av01.pok.ibm.com (8.14.3/8.13.1/NCO v10.0 AVout) with ESMTP id n9JEBKJj030225 for ; Mon, 19 Oct 2009 10:11:20 -0400 Message-ID: <4ADC7385.1080508@us.ibm.com> Date: Mon, 19 Oct 2009 09:11:17 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH v0 0/9] QError References: <1255453026-18637-1-git-send-email-lcapitulino@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kraxel@redhat.com, qemu-devel@nongnu.org, Luiz Capitulino Markus Armbruster wrote: > I just caught up with this topic, and since there's no obvious one > message to reply to, I just reply here. > > I agree with Anthony we better get the error reporting protocol > approximately right the first time, and rushing it could lead to tears > later on. Mind, I said "approximately right", not "perfect". > > However, there are more than one way to screw this up. One is certainly > to fall short of client requirements in a way that is costly to fix > (think incompatible protocol revision). Another is to overshoot them in > a way that is costly to maintain. A third one is to spend too much time > on figuring out the perfect solution. > > I believe our true problem is that we're still confused and/or > disagreeing on client requirements, and this has led to a design that > tries to cover all the conceivable bases, and feels overengineered to > me. > > There are only so many complexity credits you can spend in a program, > both globally and individually. I'm very, very wary of making error > reporting more complex than absolutely, desperately necessary. > Reporting errors should remain as easy as we can make it, for reasons > that have already been mentioned by me and others: > > * The more cumbersome it is to report an error, the less it is done, and > the more vaguely it is done. If you have to edit more than the error > site to report an error accurately, then chances skyrocket that it > won't be reported, or it'll be reported inaccurately. And not because > coders are stupid or lazy, but because they make sensible use of their > very limited complexity credits: if you can either get the job done > with lousy error messages, or not get it done at all, guess what the > smart choice is. > > * It's highly desirable for errors to do double duty as documentation. > Code like this (random pick) doesn't need a comment: > > if (qemu_opt_get(opts, "vlan")) { > qemu_error("The 'vlan' parameter is not valid with -netdev\n"); > return -1; > } > Or: if (qemu_opt_get(opts, "vlan")) { throw InvalidParameter("The 'vlan' parameter is not valid with -netdev"); } Which would become: #define INVALID_PARAMETER "InvalidParameter" if (qemu_opt_get(opts, "vlan")) { qemu_error(INVALID_PARAMETER, "The 'vlan' parameter is not valid with -netdev"); return -1; } And if we find that this is a common occurrance, we probably should do: invalid_parameter("vlan", "-netdev"); Which would expand to: asprintf(&buf, "The '%s' parameter is not valid with %s", param, option); qemu_error_full("InvalidParameter", "{'param': %s, 'option': %s, 'desc': %s}", param, option, desc); The real fundamental requirements are: 1) In order to support high level languages, an error class is needed to generate proper exceptions. 2) To integrate well into high level languages exception mechanisms, we make errors first class objects. 3) Exceptions should have a string representation. desc is a standard string representation. There's really nothing complex here. -- Regards, Anthony Liguori