From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36292) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4kQV-0005nm-TP for qemu-devel@nongnu.org; Fri, 04 Dec 2015 02:05:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4kQU-00041A-NV for qemu-devel@nongnu.org; Fri, 04 Dec 2015 02:05:07 -0500 Received: from mail-wm0-x22d.google.com ([2a00:1450:400c:c09::22d]:33628) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4kQU-0003zv-Hm for qemu-devel@nongnu.org; Fri, 04 Dec 2015 02:05:06 -0500 Received: by wmec201 with SMTP id c201so60253225wme.0 for ; Thu, 03 Dec 2015 23:05:05 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <1449119286-57280-1-git-send-email-yanmiaobest@gmail.com> <566066EB.50101@redhat.com> Date: Fri, 4 Dec 2015 15:05:05 +0800 Message-ID: From: =?UTF-8?B?6ZiO5re8?= Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] net/vmxnet3.c: fix a build error when enabling debug output List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , Dmitry Fleytman , jasowang@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com 2015-12-04 14:58 GMT+08:00 =E9=98=8E=E6=B7=BC : > 2015-12-03 23:59 GMT+08:00 Eric Blake : >> On 12/02/2015 10:08 PM, Miao Yan wrote: >>> Macro MAC_FMT and MAC_ARG are not defined, but used in vmxnet3_net_init= (). >>> This will cause build error when debug level is raised in >>> vmxnet3_debug.h (enable all VMXNET3_DEBUG_xxx). >>> >>> Use VMXNET_MF and VXMNET_MA instead. >>> >>> Signed-off-by: Miao Yan >>> --- >>> hw/net/vmxnet3.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c >>> index 5e3a233..ea3d9b7 100644 >>> --- a/hw/net/vmxnet3.c >>> +++ b/hw/net/vmxnet3.c >>> @@ -2044,7 +2044,7 @@ static void vmxnet3_net_init(VMXNET3State *s) >>> >>> s->link_status_and_speed =3D VMXNET3_LINK_SPEED | VMXNET3_LINK_STA= TUS_UP; >>> >>> - VMW_CFPRN("Permanent MAC: " MAC_FMT, MAC_ARG(s->perm_mac.a)); >>> + VMW_CFPRN("Permanent MAC: " VMXNET_MF, VMXNET_MA(s->perm_mac.a)); >> >> This is a classic example of why dead code debug statements are evil. >> >> You should consider also providing a patch to hw/net/vmxnet_debug.h to >> fix ALL of the broken debug macros in that file to instead use a sane >> pattern, so that the format string is ALWAYS compiled and just optimized >> out when debugging is disabled. >> >> Here's a conversion of one of the macros for an example of what to do: > > > Thanks for the review. Will prepare a patch for it . Oops, cc list dropped by mistake. Add them back. > > > >> >> Instead of: >> >>> #ifdef VMXNET_DEBUG_CONFIG >>> #define VMW_CFPRN(fmt, ...) = \ >>> do { = \ >>> printf("[%s][CF][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__,= \ >>> ## __VA_ARGS__); = \ >>> } while (0) >>> #else >>> #define VMW_CFPRN(fmt, ...) do {} while (0) >>> #endif >> >> you should do: >> >> #ifdef VMXNET_DEBUG_CONFIG >> # define VMXNET_DEBUG_CONFIG_FLAG 1 >> #else >> # define VMXNET_DEBUG_CONFIG_FLAG 0 >> #endif >> #define VMW_CFPRN(fmt, ...) \ >> do { \ >> if (VMXNET_DEBUG_CONFIG_FLAG) { \ >> printf("[%s][CF][%s]: " fmt "\n", VMXNET_DEVICE_NAME, \ >> __func__, ## __VA_ARGS__); \ >> } \ >> } while (0); >> >> With that pattern, VMW_CFPRN() will now always check that its arguments >> can compile, even though it has no impact to the code size when >> VMXNET_DEBUG_CONFIG is not defined. Note that once you repair all of >> the broken macros (I count 8 in that file), you may have some fallout of >> other broken dead code that needs to be fixed. >> >> -- >> Eric Blake eblake redhat com +1-919-301-3266 >> Libvirt virtualization library http://libvirt.org >>