From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40679) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dX1rA-000425-Hb for qemu-devel@nongnu.org; Mon, 17 Jul 2017 04:58:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dX1r7-00015Q-EB for qemu-devel@nongnu.org; Mon, 17 Jul 2017 04:58:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38556) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dX1r7-00013z-4h for qemu-devel@nongnu.org; Mon, 17 Jul 2017 04:58:17 -0400 Date: Mon, 17 Jul 2017 09:58:04 +0100 From: "Daniel P. Berrange" Message-ID: <20170717085804.GC3640@redhat.com> Reply-To: "Daniel P. Berrange" References: <20170713110237.6712-1-lprosek@redhat.com> <20170713110237.6712-2-lprosek@redhat.com> <20170713133206.GA18623@stefanha-x1.localdomain> <20170714104127.GA28095@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v2 1/8] qemu-error: introduce error_report_nolf List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ladi Prosek Cc: Stefan Hajnoczi , Fernando Casas =?utf-8?B?U2Now7Zzc293?= , "Michael S. Tsirkin" , qemu-devel , Jason Wang , Markus Armbruster , Greg Kurz , arei.gonglei@huawei.com, aneesh.kumar@linux.vnet.ibm.com On Mon, Jul 17, 2017 at 08:54:18AM +0200, Ladi Prosek wrote: > On Fri, Jul 14, 2017 at 12:41 PM, Daniel P. Berrange > wrote: > > On Thu, Jul 13, 2017 at 02:32:06PM +0100, Stefan Hajnoczi wrote: > >> On Thu, Jul 13, 2017 at 01:02:30PM +0200, Ladi Prosek wrote: > >> > +/* > >> > + * Print an error message to current monitor if we have one, else to stderr. > >> > + * Format arguments like sprintf(). The resulting message should be a > >> > + * single phrase, with no trailing punctuation. The no-LF version allows > >> > + * additional text to be appended with error_printf() or error_vprintf(). > >> > + * Make sure to always close with a newline after all text is printed. > >> > + * Prepends the current location. > >> > + * It's wrong to call this in a QMP monitor. Use error_setg() there. > >> > + */ > >> > +void error_report_nolf(const char *fmt, ...) > >> > +{ > >> > + va_list ap; > >> > + > >> > + va_start(ap, fmt); > >> > + error_vreport_nolf(fmt, ap); > >> > + va_end(ap); > >> > } > >> > >> Each call to this function prepends the timestamp, so it cannot really > >> be used for a sequence of prints in a single line. > >> > >> It's a little ugly but I expected something along the lines of > >> g_strdup_vprintf() in virtio_error(): > >> > >> char *msg; > >> > >> va_start(ap, fmt); > >> msg = g_strdup_vprintf(fmt, ap); > >> va_end(ap); > >> > >> error_report("%s: %s", DEVICE(vdev)->id, msg); > >> > >> g_free(msg); > > > > You could get the same thing by turning virtio_Error into a macro with > > a few games. Rename the current method to virtio_error_impl() and then > > define: > > > > #define virtio_error(dev, fmt, ...) \ > > virtio_error_impl(dev, "%s: " fmt, DEVICE(dev)->id, __VA_ARGS__) > > Neat! I think I'll stick with a function though. This doesn't allocate > but it adds a little bit of code to each call site which has the > potential of slowing down the fast no-error path (I have no data, just > the general keeping-the-code-compact-is-good principle). Holler if you > disagree! IMHO that would be uneccessary optimization, particular since this is in the error scenario and so is not performance critical to normal operation. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|