* [Qemu-devel] [PATCH] net/vmxnet3.c: fix a build error when enabling debug output @ 2015-12-03 5:08 Miao Yan 2015-12-03 7:24 ` Dmitry Fleytman 2015-12-03 15:59 ` Eric Blake 0 siblings, 2 replies; 8+ messages in thread From: Miao Yan @ 2015-12-03 5:08 UTC (permalink / raw) To: jasowang, dmitry, qemu-devel 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 = 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] net/vmxnet3.c: fix a build error when enabling debug output 2015-12-03 5:08 [Qemu-devel] [PATCH] net/vmxnet3.c: fix a build error when enabling debug output Miao Yan @ 2015-12-03 7:24 ` Dmitry Fleytman 2015-12-04 3:14 ` Jason Wang 2015-12-03 15:59 ` Eric Blake 1 sibling, 1 reply; 8+ messages in thread From: Dmitry Fleytman @ 2015-12-03 7:24 UTC (permalink / raw) To: Miao Yan; +Cc: jasowang, qemu-devel [-- Attachment #1: Type: text/plain, Size: 1079 bytes --] Acked-by: Dmitry Fleytman <dmitry@daynix.com <mailto: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 = 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 > [-- Attachment #2: Type: text/html, Size: 2129 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] net/vmxnet3.c: fix a build error when enabling debug output 2015-12-03 7:24 ` Dmitry Fleytman @ 2015-12-04 3:14 ` Jason Wang 0 siblings, 0 replies; 8+ messages in thread From: Jason Wang @ 2015-12-04 3:14 UTC (permalink / raw) To: Dmitry Fleytman, Miao Yan; +Cc: qemu-devel On 12/03/2015 03:24 PM, Dmitry Fleytman wrote: > Acked-by: Dmitry Fleytman <dmitry@daynix.com <mailto:dmitry@daynix.com>> Applied in my -net for 2.5. Thanks > >> On 3 Dec 2015, at 07:08 AM, Miao Yan <yanmiaobest@gmail.com >> <mailto: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 >> <mailto: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 = 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 >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] net/vmxnet3.c: fix a build error when enabling debug output 2015-12-03 5:08 [Qemu-devel] [PATCH] net/vmxnet3.c: fix a build error when enabling debug output Miao Yan 2015-12-03 7:24 ` Dmitry Fleytman @ 2015-12-03 15:59 ` Eric Blake 2015-12-03 16:40 ` Markus Armbruster [not found] ` <CAPc5xaOOb-FAxzXPgVrtOmqJvSs6Zwk7Rvy5qy3kFFMsnMF+2A@mail.gmail.com> 1 sibling, 2 replies; 8+ messages in thread From: Eric Blake @ 2015-12-03 15:59 UTC (permalink / raw) To: Miao Yan, jasowang, dmitry, qemu-devel [-- Attachment #1: Type: text/plain, Size: 2729 bytes --] 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 <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 = 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. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] net/vmxnet3.c: fix a build error when enabling debug output 2015-12-03 15:59 ` Eric Blake @ 2015-12-03 16:40 ` Markus Armbruster 2015-12-04 10:24 ` 阎淼 [not found] ` <CAPc5xaOOb-FAxzXPgVrtOmqJvSs6Zwk7Rvy5qy3kFFMsnMF+2A@mail.gmail.com> 1 sibling, 1 reply; 8+ messages in thread From: Markus Armbruster @ 2015-12-03 16:40 UTC (permalink / raw) To: Eric Blake; +Cc: dmitry, Miao Yan, jasowang, qemu-devel Eric Blake <eblake@redhat.com> 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 <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 = 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] net/vmxnet3.c: fix a build error when enabling debug output 2015-12-03 16:40 ` Markus Armbruster @ 2015-12-04 10:24 ` 阎淼 2015-12-04 12:07 ` Markus Armbruster 0 siblings, 1 reply; 8+ messages in thread From: 阎淼 @ 2015-12-04 10:24 UTC (permalink / raw) To: Markus Armbruster; +Cc: Dmitry Fleytman, jasowang, qemu-devel 2015-12-04 0:40 GMT+08:00 Markus Armbruster <armbru@redhat.com>: > Eric Blake <eblake@redhat.com> 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 <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 = 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] net/vmxnet3.c: fix a build error when enabling debug output 2015-12-04 10:24 ` 阎淼 @ 2015-12-04 12:07 ` Markus Armbruster 0 siblings, 0 replies; 8+ messages in thread From: Markus Armbruster @ 2015-12-04 12:07 UTC (permalink / raw) To: 阎淼; +Cc: Dmitry Fleytman, jasowang, qemu-devel 阎淼 <yanmiaobest@gmail.com> writes: > 2015-12-04 0:40 GMT+08:00 Markus Armbruster <armbru@redhat.com>: >> Eric Blake <eblake@redhat.com> 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 <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 = 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 ? 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <CAPc5xaOOb-FAxzXPgVrtOmqJvSs6Zwk7Rvy5qy3kFFMsnMF+2A@mail.gmail.com>]
* Re: [Qemu-devel] [PATCH] net/vmxnet3.c: fix a build error when enabling debug output [not found] ` <CAPc5xaOOb-FAxzXPgVrtOmqJvSs6Zwk7Rvy5qy3kFFMsnMF+2A@mail.gmail.com> @ 2015-12-04 7:05 ` 阎淼 0 siblings, 0 replies; 8+ messages in thread From: 阎淼 @ 2015-12-04 7:05 UTC (permalink / raw) To: Eric Blake, Dmitry Fleytman, jasowang, qemu-devel, armbru 2015-12-04 14:58 GMT+08:00 阎淼 <yanmiaobest@gmail.com>: > 2015-12-03 23:59 GMT+08:00 Eric Blake <eblake@redhat.com>: >> 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 <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 = 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: > > > 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 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-12-04 12:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-03 5:08 [Qemu-devel] [PATCH] net/vmxnet3.c: fix a build error when enabling debug output Miao Yan
2015-12-03 7:24 ` Dmitry Fleytman
2015-12-04 3:14 ` Jason Wang
2015-12-03 15:59 ` Eric Blake
2015-12-03 16:40 ` Markus Armbruster
2015-12-04 10:24 ` 阎淼
2015-12-04 12:07 ` Markus Armbruster
[not found] ` <CAPc5xaOOb-FAxzXPgVrtOmqJvSs6Zwk7Rvy5qy3kFFMsnMF+2A@mail.gmail.com>
2015-12-04 7:05 ` 阎淼
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).