From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36264) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bi2KA-00024Q-BH for qemu-devel@nongnu.org; Thu, 08 Sep 2016 12:37:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bi2K6-0000QW-3Z for qemu-devel@nongnu.org; Thu, 08 Sep 2016 12:37:13 -0400 References: <20160829171021.4902-1-pbutsykin@virtuozzo.com> <20160829171021.4902-5-pbutsykin@virtuozzo.com> <663cb170-71b3-3ca1-ce61-b8b8defa6147@redhat.com> <57D1888C.1070905@virtuozzo.com> From: Pavel Butsykin Message-ID: <57D18C2D.8070903@virtuozzo.com> Date: Thu, 8 Sep 2016 19:05:01 +0300 MIME-Version: 1.0 In-Reply-To: <57D1888C.1070905@virtuozzo.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC v2 04/22] block/pcache: add pcache debug build List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-block@nongnu.org, qemu-devel@nongnu.org Cc: kwolf@redhat.com, mreitz@redhat.com, stefanha@redhat.com, den@openvz.org, jsnow@redhat.com, famz@redhat.com On 08.09.2016 18:49, Pavel Butsykin wrote: > On 08.09.2016 18:11, Eric Blake wrote: >> On 08/29/2016 12:10 PM, Pavel Butsykin wrote: >>> Signed-off-by: Pavel Butsykin >>> --- >>> block/pcache.c | 9 +++++++++ >>> 1 file changed, 9 insertions(+) >>> >>> diff --git a/block/pcache.c b/block/pcache.c >>> index 74a4bc4..7f221d6 100644 >>> --- a/block/pcache.c >>> +++ b/block/pcache.c >>> @@ -28,6 +28,15 @@ >>> #include "qapi/error.h" >>> #include "qapi/qmp/qstring.h" >>> >>> +#define PCACHE_DEBUG >> >> Are you sure you want this left enabled? > > No. > >>> + >>> +#ifdef PCACHE_DEBUG >>> +#define DPRINTF(fmt, ...) \ >>> + printf("%s:%s:%d "fmt, __FILE__, __func__, __LINE__, ## >>> __VA_ARGS__) >>> +#else >>> +#define DPRINTF(fmt, ...) do { } while (0) >>> +#endif >> >> NACK. This leads to bitrot when PCACHE_DEBUG is not defined. Also, we >> typically send debug to stderr, not stdout. Instead, please follow the >> lead of many other debug places, which do something similar to this (off >> the top of my head, therefore untested): >> >> #ifdef PCACHE_DEBUG >> # define PCACHE_DEBUG_PRINT 1 >> #else >> # define PCACHE_DEBUG_PRINT 0 >> #endif >> #define DPRINTF(fmt, ...) \ >> do { \ >> if (PCACHE_DEBUG_PRINT) { \ >> fprintf(stderr, ... __VA_ARGS__) \ >> } \ >> } while (0) >> > > OK, thanks! Can I replace DPRINTFs on tracepoints?