From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51047) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9xo1-0007B4-Eh for qemu-devel@nongnu.org; Fri, 18 Dec 2015 11:22:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a9xo0-0006Tb-Mf for qemu-devel@nongnu.org; Fri, 18 Dec 2015 11:22:57 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49189) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9xo0-0006TV-Hw for qemu-devel@nongnu.org; Fri, 18 Dec 2015 11:22:56 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 149AEF804C for ; Fri, 18 Dec 2015 16:22:56 +0000 (UTC) From: Markus Armbruster References: <1450452927-8346-1-git-send-email-armbru@redhat.com> <1450452927-8346-12-git-send-email-armbru@redhat.com> <56742F6C.10408@redhat.com> Date: Fri, 18 Dec 2015 17:22:53 +0100 In-Reply-To: <56742F6C.10408@redhat.com> (Eric Blake's message of "Fri, 18 Dec 2015 09:08:12 -0700") Message-ID: <87a8p720bm.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v3 11/24] error: Use error_reportf_err() where it makes obvious sense List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org Eric Blake writes: > On 12/18/2015 08:35 AM, Markus Armbruster wrote: >> Done with this Coccinelle semantic patch >> >> @@ >> expression FMT, E, S; >> expression list ARGS; >> @@ >> - error_report(FMT, ARGS, error_get_pretty(E)); >> + error_reportf_err(E, FMT/*@@@*/, ARGS); >> ( >> - error_free(E); >> | >> exit(S); >> | >> abort(); >> ) >> >> followed by a replace of '%s"/*@@@*/' by '"' and some line rewrapping, >> because I can't figure out how to make Coccinelle transform strings. >> >> We now use the error whole instead of just its message obtained with >> error_get_pretty(). This avoids suppressing its hint (see commit >> 50b7b00), but I can't see how the errors touched in this commit could >> come with hints. >> >> Signed-off-by: Markus Armbruster >> --- > >> +++ b/arch_init.c >> @@ -258,9 +258,7 @@ void do_acpitable_option(const QemuOpts *opts) >> >> acpi_table_add(opts, &err); >> if (err) { >> - error_report("Wrong acpi table provided: %s", >> - error_get_pretty(err)); >> - error_free(err); >> + error_reportf_err(err, "Wrong acpi table provided: "); > > Bikeshedding: should error_reportf_err() automatically add the trailing > ": " to the prefix, instead of having every caller express it? Would > affect 10/24 as well. But I can't see a strong reason to add the churn > it would cause for a respin, so I won't insist. I decided not to add ": " automatically, to make the prefix-ness of the argument a bit more obvious. > Reviewed-by: Eric Blake Thanks!