From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Nm8Bs-000745-77 for qemu-devel@nongnu.org; Mon, 01 Mar 2010 11:09:52 -0500 Received: from [199.232.76.173] (port=58885 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Nm8Br-00073m-Tm for qemu-devel@nongnu.org; Mon, 01 Mar 2010 11:09:51 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1Nm8Bq-0007Vn-5c for qemu-devel@nongnu.org; Mon, 01 Mar 2010 11:09:51 -0500 Received: from mx1.redhat.com ([209.132.183.28]:5811) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Nm8Bp-0007Vh-Lz for qemu-devel@nongnu.org; Mon, 01 Mar 2010 11:09:50 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o21G9mA2015127 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 1 Mar 2010 11:09:48 -0500 Date: Mon, 1 Mar 2010 13:09:41 -0300 From: Luiz Capitulino Subject: Re: [Qemu-devel] [PATCH RFC 12/48] error: New error_printf() and error_vprintf() Message-ID: <20100301130941.34e28a8c@redhat.com> In-Reply-To: References: <1267034160-3517-1-git-send-email-armbru@redhat.com> <1267034160-3517-13-git-send-email-armbru@redhat.com> <20100226164354.6be8466d@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org On Mon, 01 Mar 2010 09:54:32 +0100 Markus Armbruster wrote: > Luiz Capitulino writes: > > > On Wed, 24 Feb 2010 18:55:24 +0100 > > Markus Armbruster wrote: > > > >> FIXME They should return int, so callers can calculate width. > >> > >> Signed-off-by: Markus Armbruster > >> --- > >> qemu-error.c | 49 ++++++++++++++++++++++++++++++++++++++++++------- > >> qemu-error.h | 14 ++++++++++++++ > >> 2 files changed, 56 insertions(+), 7 deletions(-) > >> > >> diff --git a/qemu-error.c b/qemu-error.c > >> index 63bcdcf..76c660a 100644 > >> --- a/qemu-error.c > >> +++ b/qemu-error.c > >> @@ -1,18 +1,53 @@ > >> +/* > >> + * Error reporting > >> + * > >> + * Copyright (C) 2010 Red Hat Inc. > >> + * > >> + * Authors: > >> + * Markus Armbruster , > >> + * > >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. > >> + * See the COPYING file in the top-level directory. > >> + */ > >> + > >> #include > >> #include "monitor.h" > >> #include "sysemu.h" > >> > >> -void qemu_error(const char *fmt, ...) > >> +/* > >> + * Print to current monitor if we have one, else to stderr. > >> + * FIXME should return int, so callers can calculate width, but that > >> + * requires surgery to monitor_printf(). Left for another day. > >> + */ > >> +void error_vprintf(const char *fmt, va_list ap) > >> { > >> - va_list args; > >> - > >> - va_start(args, fmt); > >> if (cur_mon) { > >> - monitor_vprintf(cur_mon, fmt, args); > >> + monitor_vprintf(cur_mon, fmt, ap); > >> } else { > >> - vfprintf(stderr, fmt, args); > >> + vfprintf(stderr, fmt, ap); > >> } > >> - va_end(args); > >> +} > > > > This can be static. > > Yes. But why would that be useful? It's neither a name space pollution > nor does it poke a hole into an abstraction. Well, IMHO unused public symbols serve only one purpose: to pollute the global namespace :) So, I think the question is: if it doesn't have any user and if you don't expect it to be used anytime soon: why make it public? > >> + > >> +/* > >> + * Print to current monitor if we have one, else to stderr. > >> + * FIXME just like error_vprintf() > >> + */ > >> +void error_printf(const char *fmt, ...) > >> +{ > >> + va_list ap; > >> + > >> + va_start(ap, fmt); > >> + error_vprintf(fmt, ap); > >> + va_end(ap); > >> +} > > > > This function's name is inconsistent with qemu_error() and > > qemu_error_new(). > > I'm fond of prepending qemu_ to random symbols left and right. Yes, I > know I'm reading QEMU source code, thank you :) > > If the names here are really important: What about stripping qemu_ from > qemu_error() & friends? I'm ok with that (and Paolo gave some suggestions), but I hope you submit a patch soon. It's ok to criticize/improve bad consistency policies, but it's not ok to break them.