From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37984) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4OFh-00061n-7q for qemu-devel@nongnu.org; Thu, 03 Dec 2015 02:24:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4OFe-0000ai-4k for qemu-devel@nongnu.org; Thu, 03 Dec 2015 02:24:29 -0500 Received: from mail-wm0-x231.google.com ([2a00:1450:400c:c09::231]:33746) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4OFd-0000a8-Rx for qemu-devel@nongnu.org; Thu, 03 Dec 2015 02:24:26 -0500 Received: by wmec201 with SMTP id c201so12874479wme.0 for ; Wed, 02 Dec 2015 23:24:25 -0800 (PST) Content-Type: multipart/alternative; boundary="Apple-Mail=_78EE7CF2-76BC-4E8A-84EE-F95DA74C1539" Mime-Version: 1.0 (Mac OS X Mail 9.1 \(3096.5\)) From: Dmitry Fleytman In-Reply-To: <1449119286-57280-1-git-send-email-yanmiaobest@gmail.com> Date: Thu, 3 Dec 2015 09:24:23 +0200 Message-Id: References: <1449119286-57280-1-git-send-email-yanmiaobest@gmail.com> 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: Miao Yan Cc: jasowang@redhat.com, qemu-devel@nongnu.org --Apple-Mail=_78EE7CF2-76BC-4E8A-84EE-F95DA74C1539 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=us-ascii Acked-by: Dmitry Fleytman > > On 3 Dec 2015, at 07:08 AM, 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 = VMXNET3_LINK_SPEED | VMXNET3_LINK_STATUS_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)); > > s->nic = qemu_new_nic(&net_vmxnet3_info, &s->conf, > object_get_typename(OBJECT(s)), > -- > 2.1.4 > --Apple-Mail=_78EE7CF2-76BC-4E8A-84EE-F95DA74C1539 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=us-ascii Acked-by: Dmitry Fleytman <dmitry@daynix.com>

On 3 Dec 2015, at 07:08 AM, Miao Yan <yanmiaobest@gmail.com> 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 <yanmiaobest@gmail.com>
---
= 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_STATUS_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));

=     s->nic =3D = qemu_new_nic(&net_vmxnet3_info, &s->conf,
=             &n= bsp;           &nbs= p; object_get_typename(OBJECT(s)),
--
2.1.4


= --Apple-Mail=_78EE7CF2-76BC-4E8A-84EE-F95DA74C1539-- From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39025) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4M7u-00011S-7c for qemu-devel@nongnu.org; Thu, 03 Dec 2015 00:08:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4M7r-0005OZ-3J for qemu-devel@nongnu.org; Thu, 03 Dec 2015 00:08:18 -0500 Received: from mail-pf0-x22d.google.com ([2607:f8b0:400e:c00::22d]:35775) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4M7q-0005ON-UQ for qemu-devel@nongnu.org; Thu, 03 Dec 2015 00:08:15 -0500 Received: by pfu207 with SMTP id 207so4596143pfu.2 for ; Wed, 02 Dec 2015 21:08:14 -0800 (PST) From: Miao Yan Date: Wed, 2 Dec 2015 21:08:06 -0800 Message-Id: <1449119286-57280-1-git-send-email-yanmiaobest@gmail.com> Subject: [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: jasowang@redhat.com, dmitry@daynix.com, qemu-devel@nongnu.org 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 = VMXNET3_LINK_SPEED | VMXNET3_LINK_STATUS_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)); s->nic = qemu_new_nic(&net_vmxnet3_info, &s->conf, object_get_typename(OBJECT(s)), -- 2.1.4 From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56679) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4WIR-0001H3-43 for qemu-devel@nongnu.org; Thu, 03 Dec 2015 10:59:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4WIM-0001Ce-5A for qemu-devel@nongnu.org; Thu, 03 Dec 2015 10:59:51 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44037) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4WIL-0001CX-Ti for qemu-devel@nongnu.org; Thu, 03 Dec 2015 10:59:46 -0500 References: <1449119286-57280-1-git-send-email-yanmiaobest@gmail.com> From: Eric Blake Message-ID: <566066EB.50101@redhat.com> Date: Thu, 3 Dec 2015 08:59:39 -0700 MIME-Version: 1.0 In-Reply-To: <1449119286-57280-1-git-send-email-yanmiaobest@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="1oqAnBNws1li88kDrxCsxuLhtpAoCuATT" 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: Miao Yan , jasowang@redhat.com, dmitry@daynix.com, qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --1oqAnBNws1li88kDrxCsxuLhtpAoCuATT Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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). >=20 > Use VMXNET_MF and VXMNET_MA instead. >=20 > Signed-off-by: Miao Yan > --- > hw/net/vmxnet3.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) >=20 > 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) > =20 > s->link_status_and_speed =3D VMXNET3_LINK_SPEED | VMXNET3_LINK_STA= TUS_UP; > =20 > - 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: 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. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --1oqAnBNws1li88kDrxCsxuLhtpAoCuATT Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWYGbsAAoJEKeha0olJ0NqFF8H/iZkcTvcCdl84ttBwU0bA/WD 9/hFs6Vnaz/eZR/BlMLDA1r70wDDmz3290Qs+9fKhbenDsEDysCwCxMld8QFCJo8 D+YxYtosPaqmqnNwImKOQxlMk1W4tzp9F5VIbdyKsngSE64qoeu49uHe3gceXZ65 rV0bdOt1o/VSQj/x3CP0b8mM2DYZUJmRYuBXUeTfS2vcOwi5LWohoFAkruPMQDat Kk6letwAQ7uKPJBgx/Pn3Kk+pMgww1Qg9Gr36ECdx47erVYkbPR431VZLzTRGE/B NgZ8+Nn1rW/45kW0rTuEiXCG9Qk5qd5wMDu4H6DbqqRqv7FzPcNUrzT/NY8Ksn8= =o7M3 -----END PGP SIGNATURE----- --1oqAnBNws1li88kDrxCsxuLhtpAoCuATT-- From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39770) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4Wvd-0001Td-82 for qemu-devel@nongnu.org; Thu, 03 Dec 2015 11:40:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4WvX-0007sy-IN for qemu-devel@nongnu.org; Thu, 03 Dec 2015 11:40:21 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55368) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4WvX-0007su-Aa for qemu-devel@nongnu.org; Thu, 03 Dec 2015 11:40:15 -0500 From: Markus Armbruster References: <1449119286-57280-1-git-send-email-yanmiaobest@gmail.com> <566066EB.50101@redhat.com> Date: Thu, 03 Dec 2015 17:40:12 +0100 In-Reply-To: <566066EB.50101@redhat.com> (Eric Blake's message of "Thu, 3 Dec 2015 08:59:39 -0700") Message-ID: <87h9jzpjtf.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain 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 Cc: dmitry@daynix.com, Miao Yan , jasowang@redhat.com, qemu-devel@nongnu.org Eric Blake writes: > 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 = VMXNET3_LINK_SPEED | VMXNET3_LINK_STATUS_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: > > 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. You may want to consider tracepoints instead of hand-rolled debugging macros: docs/tracing.txt. From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41548) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4gpi-00006b-9Y for qemu-devel@nongnu.org; Thu, 03 Dec 2015 22:14:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4gpd-0007q4-5o for qemu-devel@nongnu.org; Thu, 03 Dec 2015 22:14:54 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51722) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4gpd-0007py-0v for qemu-devel@nongnu.org; Thu, 03 Dec 2015 22:14:49 -0500 References: <1449119286-57280-1-git-send-email-yanmiaobest@gmail.com> From: Jason Wang Message-ID: <5661051F.3020401@redhat.com> Date: Fri, 4 Dec 2015 11:14:39 +0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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: Dmitry Fleytman , Miao Yan Cc: qemu-devel@nongnu.org On 12/03/2015 03:24 PM, Dmitry Fleytman wrote: > Acked-by: Dmitry Fleytman > Applied in my -net for 2.5. Thanks > >> On 3 Dec 2015, at 07:08 AM, 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 = VMXNET3_LINK_SPEED | >> VMXNET3_LINK_STATUS_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)); >> >> s->nic = qemu_new_nic(&net_vmxnet3_info, &s->conf, >> object_get_typename(OBJECT(s)), >> -- >> 2.1.4 >> > 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 >> From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41080) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4nX5-00025E-6s for qemu-devel@nongnu.org; Fri, 04 Dec 2015 05:24:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4nX1-0006OR-BN for qemu-devel@nongnu.org; Fri, 04 Dec 2015 05:24:07 -0500 Received: from mail-wm0-x22d.google.com ([2a00:1450:400c:c09::22d]:34968) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4nX1-0006OH-5O for qemu-devel@nongnu.org; Fri, 04 Dec 2015 05:24:03 -0500 Received: by wmuu63 with SMTP id u63so55943226wmu.0 for ; Fri, 04 Dec 2015 02:24:02 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <87h9jzpjtf.fsf@blackfin.pond.sub.org> References: <1449119286-57280-1-git-send-email-yanmiaobest@gmail.com> <566066EB.50101@redhat.com> <87h9jzpjtf.fsf@blackfin.pond.sub.org> Date: Fri, 4 Dec 2015 18:24:02 +0800 Message-ID: From: =?UTF-8?B?6ZiO5re8?= Content-Type: text/plain; charset=UTF-8 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: Markus Armbruster Cc: Dmitry Fleytman , jasowang@redhat.com, qemu-devel@nongnu.org 2015-12-04 0:40 GMT+08:00 Markus Armbruster : > Eric Blake writes: > >> 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 = VMXNET3_LINK_SPEED | VMXNET3_LINK_STATUS_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: >> >> 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. > > You may want to consider tracepoints instead of hand-rolled debugging > macros: docs/tracing.txt. Hi Markus, Thanks for your suggestion. It seems trace functions must have fixed number of parameters, so, for example, VMW_PRN("abc") and VMW_PRN("aaa %d", a) would need two trace functions, right ? I looked at the code in vmxnet3.c, IMO not all debug output are suitable to be converted to trace points. Maybe I can work out a patch to fix the mmio part. Miao From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52665) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4p9E-00059M-DR for qemu-devel@nongnu.org; Fri, 04 Dec 2015 07:07:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4p94-0004ra-FE for qemu-devel@nongnu.org; Fri, 04 Dec 2015 07:07:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45968) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4p94-0004mV-8Z for qemu-devel@nongnu.org; Fri, 04 Dec 2015 07:07:26 -0500 From: Markus Armbruster References: <1449119286-57280-1-git-send-email-yanmiaobest@gmail.com> <566066EB.50101@redhat.com> <87h9jzpjtf.fsf@blackfin.pond.sub.org> Date: Fri, 04 Dec 2015 13:07:21 +0100 In-Reply-To: (=?utf-8?B?IumYjua3vCIncw==?= message of "Fri, 4 Dec 2015 18:24:02 +0800") Message-ID: <87vb8epgcm.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 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: =?utf-8?B?6ZiO5re8?= Cc: Dmitry Fleytman , jasowang@redhat.com, qemu-devel@nongnu.org =E9=98=8E=E6=B7=BC writes: > 2015-12-04 0:40 GMT+08:00 Markus Armbruster : >> Eric Blake writes: >> >>> On 12/02/2015 10:08 PM, Miao Yan wrote: >>>> Macro MAC_FMT and MAC_ARG are not defined, but used in vmxnet3_net_ini= t(). >>>> 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_ST= ATUS_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: >>> >>> 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. >> >> You may want to consider tracepoints instead of hand-rolled debugging >> macros: docs/tracing.txt. > > Hi Markus, > > Thanks for your suggestion. It seems trace functions must have > fixed number of parameters, so, for example, > VMW_PRN("abc") and VMW_PRN("aaa %d", a) would need two trace functions, > right ? The purpose of debug print helpers is to facilitate enabling and disabling (sets of related) debug prints at compile time, for debugging purposes. Tracepoints are more structured: they are meant for tracing the flow of control as it passes through points of interest. In general, you want a seperate tracepoint for each point of interest, so you can follow the flow of control in the trace. Tracepoints can be configured with various backends for various purposes beyond debugging by developers. You can ship code with tracepoints compiled in, but disabled, then enable selected tracepoints in the field for troubleshooting. > I looked at the code in vmxnet3.c, IMO not all debug output are > suitable to be converted to trace points. Maybe I can work out a > patch to fix the mmio part. I can't tell you which of your debug prints should be tracepoints. I just want to make you aware of tracepoints, so you can decide for yourself.