From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49356) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VWWHE-0007vL-Nc for qemu-devel@nongnu.org; Wed, 16 Oct 2013 14:57:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VWWH8-0003k2-O5 for qemu-devel@nongnu.org; Wed, 16 Oct 2013 14:57:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:64299) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VWWH8-0003jx-FZ for qemu-devel@nongnu.org; Wed, 16 Oct 2013 14:56:54 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r9GIuqx3031397 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 16 Oct 2013 14:56:53 -0400 Message-ID: <525EE171.20208@redhat.com> Date: Wed, 16 Oct 2013 20:56:49 +0200 From: Max Reitz MIME-Version: 1.0 References: <1381803804-4551-1-git-send-email-famz@redhat.com> In-Reply-To: <1381803804-4551-1-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] mirror: drop local_err in mirror_complete List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: kwolf@redhat.com, stefanha@redhat.com On 2013-10-15 04:23, Fam Zheng wrote: > There is errp passed in, so no need for local_err and error_propagate. > Also drop the backing_filename which is set but unused since 34b5d2c. I approve of dropping the backing_filename code, but I don't know if I like removing the error_propagate. I personally like explicit error_propagate calls in every function receiving an error code from an underlying function and propagating that as its own. There are two reasons for this: The first reason is pragmatic: There are lots of places in qemu where this is done exactly that way. In fact, I'm responsible for some myself. "Fixing" all of them is basically impossible and also unreasonable, since the worst error_propagate can do is blow up the code size. However, this is not the reason I'd object a patch doing something different (here: dropping the unused backing_filename code) and dropping "redundant" error_propagate calls along the way. The reason I object it here is that error_propagate *currently* is a no-op. But this may change in the future: I have already sent an RFC which extends error_propagate so it can generate an error backtrace if enabled through configure. If this (or something similar which extends error_propagate to do more than basically just *errp = local_err) is merged to/implemented in qemu later on, I believe we want to call error_propagate in every function that touches an error variable in order to generate a backtrace with maximum verbosity. Max