* [Qemu-devel] [PATCH v2 0/3] fix debug macro pattern for vmxnet3
@ 2015-12-05 8:54 Miao Yan
2015-12-05 8:55 ` [Qemu-devel] [PATCH v2 1/3] net/vmxnet3.c: fix a build error when enabling debug output Miao Yan
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Miao Yan @ 2015-12-05 8:54 UTC (permalink / raw)
To: jasowang, dmitry, qemu-devel, eblake, armbru
This patchset fixes debug macro pattern for vmxnet3. The old style uses
#ifdef...#else...#endif to define debug macros, as a result
the format string inside the macro will never be checked
(debug not turned on by default) and is likely to cause build
errors in the future when enabled.
Note patch 1/3 "fix a build error when enabling debug output"
has already been applied to -net tree by Jason Wang as
a separate patch, it's included here for completeness because the other two
depend on it.
Miao Yan (3):
net/vmxnet3.c: fix a build error when enabling debug output
net/vmxnet3: fix debug macro pattern for vmxnet3
net/vmxnet3: remove redundant VMW_SHPRN(...) definition
hw/net/vmware_utils.h | 5 +-
hw/net/vmxnet3.c | 2 +-
hw/net/vmxnet_debug.h | 139 +++++++++++++++++++++++++++++++-------------------
3 files changed, 88 insertions(+), 58 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH v2 1/3] net/vmxnet3.c: fix a build error when enabling debug output 2015-12-05 8:54 [Qemu-devel] [PATCH v2 0/3] fix debug macro pattern for vmxnet3 Miao Yan @ 2015-12-05 8:55 ` Miao Yan 2015-12-05 16:32 ` Eric Blake 2015-12-05 8:55 ` [Qemu-devel] [PATCH v2 2/3] net/vmxnet3: fix debug macro pattern for vmxnet3 Miao Yan 2015-12-05 8:55 ` [Qemu-devel] [PATCH v2 3/3] net/vmxnet3: remove redundant VMW_SHPRN(...) definition Miao Yan 2 siblings, 1 reply; 9+ messages in thread From: Miao Yan @ 2015-12-05 8:55 UTC (permalink / raw) To: jasowang, dmitry, qemu-devel, eblake, armbru 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)), -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/3] net/vmxnet3.c: fix a build error when enabling debug output 2015-12-05 8:55 ` [Qemu-devel] [PATCH v2 1/3] net/vmxnet3.c: fix a build error when enabling debug output Miao Yan @ 2015-12-05 16:32 ` Eric Blake 0 siblings, 0 replies; 9+ messages in thread From: Eric Blake @ 2015-12-05 16:32 UTC (permalink / raw) To: Miao Yan, jasowang, dmitry, qemu-devel, armbru [-- Attachment #1: Type: text/plain, Size: 1151 bytes --] On 12/05/2015 01:55 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 <yanmiaobest@gmail.com> > --- > hw/net/vmxnet3.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Eric Blake <eblake@redhat.com> > > 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)), > -- 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] 9+ messages in thread
* [Qemu-devel] [PATCH v2 2/3] net/vmxnet3: fix debug macro pattern for vmxnet3 2015-12-05 8:54 [Qemu-devel] [PATCH v2 0/3] fix debug macro pattern for vmxnet3 Miao Yan 2015-12-05 8:55 ` [Qemu-devel] [PATCH v2 1/3] net/vmxnet3.c: fix a build error when enabling debug output Miao Yan @ 2015-12-05 8:55 ` Miao Yan 2015-12-07 2:47 ` Jason Wang 2015-12-05 8:55 ` [Qemu-devel] [PATCH v2 3/3] net/vmxnet3: remove redundant VMW_SHPRN(...) definition Miao Yan 2 siblings, 1 reply; 9+ messages in thread From: Miao Yan @ 2015-12-05 8:55 UTC (permalink / raw) To: jasowang, dmitry, qemu-devel, eblake, armbru Vmxnet3 uses the following debug macro style: #ifdef SOME_DEBUG # define debug(...) do{ printf(...); } while (0) # else # define debug(...) do{ } while (0) #endif If SOME_DEBUG is undefined, then format string inside the debug macro will never be checked by compiler. Code is likely to break in the future when SOME_DEBUG is enabled because of lack of testing. This patch changes this to the following: #define debug(...) \ do { if (SOME_DEBUG_ENABLED) printf(...); } while (0) Signed-off-by: Miao Yan <yanmiaobest@gmail.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- Changes in v2: - Fix grammar error in commit log hw/net/vmxnet_debug.h | 139 +++++++++++++++++++++++++++++++------------------- 1 file changed, 86 insertions(+), 53 deletions(-) diff --git a/hw/net/vmxnet_debug.h b/hw/net/vmxnet_debug.h index 96dae0f..96495db 100644 --- a/hw/net/vmxnet_debug.h +++ b/hw/net/vmxnet_debug.h @@ -20,94 +20,127 @@ #define VMXNET_DEVICE_NAME "vmxnet3" -/* #define VMXNET_DEBUG_CB */ #define VMXNET_DEBUG_WARNINGS #define VMXNET_DEBUG_ERRORS -/* #define VMXNET_DEBUG_INTERRUPTS */ -/* #define VMXNET_DEBUG_CONFIG */ -/* #define VMXNET_DEBUG_RINGS */ -/* #define VMXNET_DEBUG_PACKETS */ -/* #define VMXNET_DEBUG_SHMEM_ACCESS */ + +#undef VMXNET_DEBUG_CB +#undef VMXNET_DEBUG_INTERRUPTS +#undef VMXNET_DEBUG_CONFIG +#undef VMXNET_DEBUG_RINGS +#undef VMXNET_DEBUG_PACKETS +#undef VMXNET_DEBUG_SHMEM_ACCESS + +#ifdef VMXNET_DEBUG_CB +# define VMXNET_DEBUG_CB_ENABLED 1 +#else +# define VMXNET_DEBUG_CB_ENABLED 0 +#endif + +#ifdef VMXNET_DEBUG_WARNINGS +# define VMXNET_DEBUG_WARNINGS_ENABLED 1 +#else +# define VMXNET_DEBUG_WARNINGS_ENABLED 0 +#endif + +#ifdef VMXNET_DEBUG_ERRORS +# define VMXNET_DEBUG_ERRORS_ENABLED 1 +#else +# define VMXNET_DEBUG_ERRORS_ENABLED 0 +#endif + +#ifdef VMXNET_DEBUG_CONFIG +# define VMXNET_DEBUG_CONFIG_ENABLED 1 +#else +# define VMXNET_DEBUG_CONFIG_ENABLED 0 +#endif + +#ifdef VMXNET_DEBUG_RINGS +# define VMXNET_DEBUG_RINGS_ENABLED 1 +#else +# define VMXNET_DEBUG_RINGS_ENABLED 0 +#endif + +#ifdef VMXNET_DEBUG_PACKETS +# define VMXNET_DEBUG_PACKETS_ENABLED 1 +#else +# define VMXNET_DEBUG_PACKETS_ENABLED 0 +#endif + +#ifdef VMXNET_DEBUG_INTERRUPTS +# define VMXNET_DEBUG_INTERRUPTS_ENABLED 1 +#else +# define VMXNET_DEBUG_INTERRUPTS_ENABLED 0 +#endif #ifdef VMXNET_DEBUG_SHMEM_ACCESS +# define VMXNET_DEBUG_SHMEM_ACCESS_ENABLED 1 +#else +# define VMXNET_DEBUG_SHMEM_ACCESS_ENABLED 0 +#endif + #define VMW_SHPRN(fmt, ...) \ do { \ - printf("[%s][SH][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ - ## __VA_ARGS__); \ + if (VMXNET_DEBUG_SHMEM_ACCESS_ENABLED) { \ + printf("[%s][SH][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ + ## __VA_ARGS__); \ + } \ } while (0) -#else -#define VMW_SHPRN(fmt, ...) do {} while (0) -#endif -#ifdef VMXNET_DEBUG_CB #define VMW_CBPRN(fmt, ...) \ do { \ - printf("[%s][CB][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ - ## __VA_ARGS__); \ + if (VMXNET_DEBUG_CB_ENABLED) { \ + printf("[%s][CB][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ + ## __VA_ARGS__); \ + } \ } while (0) -#else -#define VMW_CBPRN(fmt, ...) do {} while (0) -#endif -#ifdef VMXNET_DEBUG_PACKETS #define VMW_PKPRN(fmt, ...) \ do { \ - printf("[%s][PK][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ - ## __VA_ARGS__); \ + if (VMXNET_DEBUG_PACKETS_ENABLED) { \ + printf("[%s][PK][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ + ## __VA_ARGS__); \ + } \ } while (0) -#else -#define VMW_PKPRN(fmt, ...) do {} while (0) -#endif -#ifdef VMXNET_DEBUG_WARNINGS #define VMW_WRPRN(fmt, ...) \ do { \ - printf("[%s][WR][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ - ## __VA_ARGS__); \ + if (VMXNET_DEBUG_WARNINGS_ENABLED) { \ + printf("[%s][WR][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ + ## __VA_ARGS__); \ + } \ } while (0) -#else -#define VMW_WRPRN(fmt, ...) do {} while (0) -#endif -#ifdef VMXNET_DEBUG_ERRORS #define VMW_ERPRN(fmt, ...) \ do { \ - printf("[%s][ER][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ - ## __VA_ARGS__); \ + if (VMXNET_DEBUG_ERRORS_ENABLED) { \ + printf("[%s][ER][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ + ## __VA_ARGS__); \ + } \ } while (0) -#else -#define VMW_ERPRN(fmt, ...) do {} while (0) -#endif -#ifdef VMXNET_DEBUG_INTERRUPTS #define VMW_IRPRN(fmt, ...) \ do { \ - printf("[%s][IR][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ - ## __VA_ARGS__); \ + if (VMXNET_DEBUG_INTERRUPTS_ENABLED) { \ + printf("[%s][IR][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ + ## __VA_ARGS__); \ + } \ } while (0) -#else -#define VMW_IRPRN(fmt, ...) do {} while (0) -#endif -#ifdef VMXNET_DEBUG_CONFIG #define VMW_CFPRN(fmt, ...) \ do { \ - printf("[%s][CF][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ - ## __VA_ARGS__); \ + if (VMXNET_DEBUG_CONFIG_ENABLED) { \ + printf("[%s][CF][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ + ## __VA_ARGS__); \ + } \ } while (0) -#else -#define VMW_CFPRN(fmt, ...) do {} while (0) -#endif -#ifdef VMXNET_DEBUG_RINGS #define VMW_RIPRN(fmt, ...) \ do { \ - printf("[%s][RI][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ - ## __VA_ARGS__); \ + if (VMXNET_DEBUG_RINGS_ENABLED) { \ + printf("[%s][RI][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ + ## __VA_ARGS__); \ + } \ } while (0) -#else -#define VMW_RIPRN(fmt, ...) do {} while (0) -#endif #define VMXNET_MF "%02X:%02X:%02X:%02X:%02X:%02X" #define VMXNET_MA(a) (a)[0], (a)[1], (a)[2], (a)[3], (a)[4], (a)[5] -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/3] net/vmxnet3: fix debug macro pattern for vmxnet3 2015-12-05 8:55 ` [Qemu-devel] [PATCH v2 2/3] net/vmxnet3: fix debug macro pattern for vmxnet3 Miao Yan @ 2015-12-07 2:47 ` Jason Wang 2015-12-07 22:04 ` Eric Blake 0 siblings, 1 reply; 9+ messages in thread From: Jason Wang @ 2015-12-07 2:47 UTC (permalink / raw) To: Miao Yan, dmitry, qemu-devel, eblake, armbru On 12/05/2015 04:55 PM, Miao Yan wrote: > Vmxnet3 uses the following debug macro style: > > #ifdef SOME_DEBUG > # define debug(...) do{ printf(...); } while (0) > # else > # define debug(...) do{ } while (0) > #endif > > If SOME_DEBUG is undefined, then format string inside the > debug macro will never be checked by compiler. Code is > likely to break in the future when SOME_DEBUG is enabled > because of lack of testing. This patch changes this > to the following: > > #define debug(...) \ > do { if (SOME_DEBUG_ENABLED) printf(...); } while (0) > > Signed-off-by: Miao Yan <yanmiaobest@gmail.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > --- > Changes in v2: > - Fix grammar error in commit log Applied in my -net for 2.5. Thanks > > hw/net/vmxnet_debug.h | 139 +++++++++++++++++++++++++++++++------------------- > 1 file changed, 86 insertions(+), 53 deletions(-) > > diff --git a/hw/net/vmxnet_debug.h b/hw/net/vmxnet_debug.h > index 96dae0f..96495db 100644 > --- a/hw/net/vmxnet_debug.h > +++ b/hw/net/vmxnet_debug.h > @@ -20,94 +20,127 @@ > > #define VMXNET_DEVICE_NAME "vmxnet3" > > -/* #define VMXNET_DEBUG_CB */ > #define VMXNET_DEBUG_WARNINGS > #define VMXNET_DEBUG_ERRORS > -/* #define VMXNET_DEBUG_INTERRUPTS */ > -/* #define VMXNET_DEBUG_CONFIG */ > -/* #define VMXNET_DEBUG_RINGS */ > -/* #define VMXNET_DEBUG_PACKETS */ > -/* #define VMXNET_DEBUG_SHMEM_ACCESS */ > + > +#undef VMXNET_DEBUG_CB > +#undef VMXNET_DEBUG_INTERRUPTS > +#undef VMXNET_DEBUG_CONFIG > +#undef VMXNET_DEBUG_RINGS > +#undef VMXNET_DEBUG_PACKETS > +#undef VMXNET_DEBUG_SHMEM_ACCESS > + > +#ifdef VMXNET_DEBUG_CB > +# define VMXNET_DEBUG_CB_ENABLED 1 > +#else > +# define VMXNET_DEBUG_CB_ENABLED 0 > +#endif > + > +#ifdef VMXNET_DEBUG_WARNINGS > +# define VMXNET_DEBUG_WARNINGS_ENABLED 1 > +#else > +# define VMXNET_DEBUG_WARNINGS_ENABLED 0 > +#endif > + > +#ifdef VMXNET_DEBUG_ERRORS > +# define VMXNET_DEBUG_ERRORS_ENABLED 1 > +#else > +# define VMXNET_DEBUG_ERRORS_ENABLED 0 > +#endif > + > +#ifdef VMXNET_DEBUG_CONFIG > +# define VMXNET_DEBUG_CONFIG_ENABLED 1 > +#else > +# define VMXNET_DEBUG_CONFIG_ENABLED 0 > +#endif > + > +#ifdef VMXNET_DEBUG_RINGS > +# define VMXNET_DEBUG_RINGS_ENABLED 1 > +#else > +# define VMXNET_DEBUG_RINGS_ENABLED 0 > +#endif > + > +#ifdef VMXNET_DEBUG_PACKETS > +# define VMXNET_DEBUG_PACKETS_ENABLED 1 > +#else > +# define VMXNET_DEBUG_PACKETS_ENABLED 0 > +#endif > + > +#ifdef VMXNET_DEBUG_INTERRUPTS > +# define VMXNET_DEBUG_INTERRUPTS_ENABLED 1 > +#else > +# define VMXNET_DEBUG_INTERRUPTS_ENABLED 0 > +#endif > > #ifdef VMXNET_DEBUG_SHMEM_ACCESS > +# define VMXNET_DEBUG_SHMEM_ACCESS_ENABLED 1 > +#else > +# define VMXNET_DEBUG_SHMEM_ACCESS_ENABLED 0 > +#endif > + > #define VMW_SHPRN(fmt, ...) \ > do { \ > - printf("[%s][SH][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ > - ## __VA_ARGS__); \ > + if (VMXNET_DEBUG_SHMEM_ACCESS_ENABLED) { \ > + printf("[%s][SH][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ > + ## __VA_ARGS__); \ > + } \ > } while (0) > -#else > -#define VMW_SHPRN(fmt, ...) do {} while (0) > -#endif > > -#ifdef VMXNET_DEBUG_CB > #define VMW_CBPRN(fmt, ...) \ > do { \ > - printf("[%s][CB][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ > - ## __VA_ARGS__); \ > + if (VMXNET_DEBUG_CB_ENABLED) { \ > + printf("[%s][CB][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ > + ## __VA_ARGS__); \ > + } \ > } while (0) > -#else > -#define VMW_CBPRN(fmt, ...) do {} while (0) > -#endif > > -#ifdef VMXNET_DEBUG_PACKETS > #define VMW_PKPRN(fmt, ...) \ > do { \ > - printf("[%s][PK][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ > - ## __VA_ARGS__); \ > + if (VMXNET_DEBUG_PACKETS_ENABLED) { \ > + printf("[%s][PK][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ > + ## __VA_ARGS__); \ > + } \ > } while (0) > -#else > -#define VMW_PKPRN(fmt, ...) do {} while (0) > -#endif > > -#ifdef VMXNET_DEBUG_WARNINGS > #define VMW_WRPRN(fmt, ...) \ > do { \ > - printf("[%s][WR][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ > - ## __VA_ARGS__); \ > + if (VMXNET_DEBUG_WARNINGS_ENABLED) { \ > + printf("[%s][WR][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ > + ## __VA_ARGS__); \ > + } \ > } while (0) > -#else > -#define VMW_WRPRN(fmt, ...) do {} while (0) > -#endif > > -#ifdef VMXNET_DEBUG_ERRORS > #define VMW_ERPRN(fmt, ...) \ > do { \ > - printf("[%s][ER][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ > - ## __VA_ARGS__); \ > + if (VMXNET_DEBUG_ERRORS_ENABLED) { \ > + printf("[%s][ER][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ > + ## __VA_ARGS__); \ > + } \ > } while (0) > -#else > -#define VMW_ERPRN(fmt, ...) do {} while (0) > -#endif > > -#ifdef VMXNET_DEBUG_INTERRUPTS > #define VMW_IRPRN(fmt, ...) \ > do { \ > - printf("[%s][IR][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ > - ## __VA_ARGS__); \ > + if (VMXNET_DEBUG_INTERRUPTS_ENABLED) { \ > + printf("[%s][IR][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ > + ## __VA_ARGS__); \ > + } \ > } while (0) > -#else > -#define VMW_IRPRN(fmt, ...) do {} while (0) > -#endif > > -#ifdef VMXNET_DEBUG_CONFIG > #define VMW_CFPRN(fmt, ...) \ > do { \ > - printf("[%s][CF][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ > - ## __VA_ARGS__); \ > + if (VMXNET_DEBUG_CONFIG_ENABLED) { \ > + printf("[%s][CF][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ > + ## __VA_ARGS__); \ > + } \ > } while (0) > -#else > -#define VMW_CFPRN(fmt, ...) do {} while (0) > -#endif > > -#ifdef VMXNET_DEBUG_RINGS > #define VMW_RIPRN(fmt, ...) \ > do { \ > - printf("[%s][RI][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ > - ## __VA_ARGS__); \ > + if (VMXNET_DEBUG_RINGS_ENABLED) { \ > + printf("[%s][RI][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, \ > + ## __VA_ARGS__); \ > + } \ > } while (0) > -#else > -#define VMW_RIPRN(fmt, ...) do {} while (0) > -#endif > > #define VMXNET_MF "%02X:%02X:%02X:%02X:%02X:%02X" > #define VMXNET_MA(a) (a)[0], (a)[1], (a)[2], (a)[3], (a)[4], (a)[5] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/3] net/vmxnet3: fix debug macro pattern for vmxnet3 2015-12-07 2:47 ` Jason Wang @ 2015-12-07 22:04 ` Eric Blake 2015-12-08 2:53 ` Miao Yan 2015-12-11 3:12 ` Jason Wang 0 siblings, 2 replies; 9+ messages in thread From: Eric Blake @ 2015-12-07 22:04 UTC (permalink / raw) To: Jason Wang, Miao Yan, dmitry, qemu-devel, armbru [-- Attachment #1: Type: text/plain, Size: 1827 bytes --] On 12/06/2015 07:47 PM, Jason Wang wrote: > > > On 12/05/2015 04:55 PM, Miao Yan wrote: >> Vmxnet3 uses the following debug macro style: >> >> #ifdef SOME_DEBUG >> # define debug(...) do{ printf(...); } while (0) >> # else >> # define debug(...) do{ } while (0) >> #endif >> >> If SOME_DEBUG is undefined, then format string inside the >> debug macro will never be checked by compiler. Code is >> likely to break in the future when SOME_DEBUG is enabled >> because of lack of testing. This patch changes this >> to the following: >> >> #define debug(...) \ >> do { if (SOME_DEBUG_ENABLED) printf(...); } while (0) >> >> Signed-off-by: Miao Yan <yanmiaobest@gmail.com> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> --- >> Changes in v2: >> - Fix grammar error in commit log > > Applied in my -net for 2.5. Thanks I don't know if Miao saw Peter's reaction to the pull request, but just as I guessed on v1, exposing more format strings turned up more latent bugs in the format strings, with failures such as: > I'm afraid this doesn't build on 32-bit due to format string errors: > > /home/petmay01/qemu/hw/net/vmxnet3.c: In function 'vmxnet3_complete_packet': > /home/petmay01/qemu/hw/net/vmxnet3.c:500:5: error: format '%lu' > expects argument of type 'long unsigned int', but argument 7 has type > 'size_t' [-Werror=format=] > VMXNET3_RING_DUMP(VMW_RIPRN, "TXC", qidx, &s->txq_descr[qidx].comp_ring); Looks like it will need a v3; and sorry that I didn't catch the problems in my review (I was just going off of whether the macro conversion was correct, and not an actual compile test to see if all the now-live format strings were always valid). -- 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] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/3] net/vmxnet3: fix debug macro pattern for vmxnet3 2015-12-07 22:04 ` Eric Blake @ 2015-12-08 2:53 ` Miao Yan 2015-12-11 3:12 ` Jason Wang 1 sibling, 0 replies; 9+ messages in thread From: Miao Yan @ 2015-12-08 2:53 UTC (permalink / raw) To: Eric Blake, peter.maydell Cc: Dmitry Fleytman, Jason Wang, qemu-devel, Markus Armbruster 2015-12-08 6:04 GMT+08:00 Eric Blake <eblake@redhat.com>: > On 12/06/2015 07:47 PM, Jason Wang wrote: >> >> >> On 12/05/2015 04:55 PM, Miao Yan wrote: >>> Vmxnet3 uses the following debug macro style: >>> >>> #ifdef SOME_DEBUG >>> # define debug(...) do{ printf(...); } while (0) >>> # else >>> # define debug(...) do{ } while (0) >>> #endif >>> >>> If SOME_DEBUG is undefined, then format string inside the >>> debug macro will never be checked by compiler. Code is >>> likely to break in the future when SOME_DEBUG is enabled >>> because of lack of testing. This patch changes this >>> to the following: >>> >>> #define debug(...) \ >>> do { if (SOME_DEBUG_ENABLED) printf(...); } while (0) >>> >>> Signed-off-by: Miao Yan <yanmiaobest@gmail.com> >>> Reviewed-by: Eric Blake <eblake@redhat.com> >>> --- >>> Changes in v2: >>> - Fix grammar error in commit log >> >> Applied in my -net for 2.5. Thanks > > I don't know if Miao saw Peter's reaction to the pull request, but just > as I guessed on v1, exposing more format strings turned up more latent > bugs in the format strings, with failures such as: Very sorry about this. > >> I'm afraid this doesn't build on 32-bit due to format string errors: >> >> /home/petmay01/qemu/hw/net/vmxnet3.c: In function 'vmxnet3_complete_packet': >> /home/petmay01/qemu/hw/net/vmxnet3.c:500:5: error: format '%lu' >> expects argument of type 'long unsigned int', but argument 7 has type >> 'size_t' [-Werror=format=] >> VMXNET3_RING_DUMP(VMW_RIPRN, "TXC", qidx, &s->txq_descr[qidx].comp_ring); I don't have a 32 bit machine for a long time, I'll create a VM to test this. > > Looks like it will need a v3; and sorry that I didn't catch the problems > in my review (I was just going off of whether the macro conversion was > correct, and not an actual compile test to see if all the now-live > format strings were always valid). I guess %zu should fix this. I'll send v3. Thanks. > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/3] net/vmxnet3: fix debug macro pattern for vmxnet3 2015-12-07 22:04 ` Eric Blake 2015-12-08 2:53 ` Miao Yan @ 2015-12-11 3:12 ` Jason Wang 1 sibling, 0 replies; 9+ messages in thread From: Jason Wang @ 2015-12-11 3:12 UTC (permalink / raw) To: Eric Blake, Miao Yan, dmitry, qemu-devel, armbru On 12/08/2015 06:04 AM, Eric Blake wrote: > On 12/06/2015 07:47 PM, Jason Wang wrote: >> >> On 12/05/2015 04:55 PM, Miao Yan wrote: >>> Vmxnet3 uses the following debug macro style: >>> >>> #ifdef SOME_DEBUG >>> # define debug(...) do{ printf(...); } while (0) >>> # else >>> # define debug(...) do{ } while (0) >>> #endif >>> >>> If SOME_DEBUG is undefined, then format string inside the >>> debug macro will never be checked by compiler. Code is >>> likely to break in the future when SOME_DEBUG is enabled >>> because of lack of testing. This patch changes this >>> to the following: >>> >>> #define debug(...) \ >>> do { if (SOME_DEBUG_ENABLED) printf(...); } while (0) >>> >>> Signed-off-by: Miao Yan <yanmiaobest@gmail.com> >>> Reviewed-by: Eric Blake <eblake@redhat.com> >>> --- >>> Changes in v2: >>> - Fix grammar error in commit log >> Applied in my -net for 2.5. Thanks > I don't know if Miao saw Peter's reaction to the pull request, but just > as I guessed on v1, exposing more format strings turned up more latent > bugs in the format strings, with failures such as: > >> I'm afraid this doesn't build on 32-bit due to format string errors: >> >> /home/petmay01/qemu/hw/net/vmxnet3.c: In function 'vmxnet3_complete_packet': >> /home/petmay01/qemu/hw/net/vmxnet3.c:500:5: error: format '%lu' >> expects argument of type 'long unsigned int', but argument 7 has type >> 'size_t' [-Werror=format=] >> VMXNET3_RING_DUMP(VMW_RIPRN, "TXC", qidx, &s->txq_descr[qidx].comp_ring); > Looks like it will need a v3; and sorry that I didn't catch the problems > in my review (I was just going off of whether the macro conversion was > correct, and not an actual compile test to see if all the now-live > format strings were always valid). > No problem, this also reminds me to do 32-bit compile test before submitting pull request. Thanks ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v2 3/3] net/vmxnet3: remove redundant VMW_SHPRN(...) definition 2015-12-05 8:54 [Qemu-devel] [PATCH v2 0/3] fix debug macro pattern for vmxnet3 Miao Yan 2015-12-05 8:55 ` [Qemu-devel] [PATCH v2 1/3] net/vmxnet3.c: fix a build error when enabling debug output Miao Yan 2015-12-05 8:55 ` [Qemu-devel] [PATCH v2 2/3] net/vmxnet3: fix debug macro pattern for vmxnet3 Miao Yan @ 2015-12-05 8:55 ` Miao Yan 2 siblings, 0 replies; 9+ messages in thread From: Miao Yan @ 2015-12-05 8:55 UTC (permalink / raw) To: jasowang, dmitry, qemu-devel, eblake, armbru Macro VMW_SHPRN(...) is already defined vmxnet3_debug.h, so remove the duplication Signed-off-by: Miao Yan <yanmiaobest@gmail.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- hw/net/vmware_utils.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/hw/net/vmware_utils.h b/hw/net/vmware_utils.h index 1099df6..c2c2f90 100644 --- a/hw/net/vmware_utils.h +++ b/hw/net/vmware_utils.h @@ -18,10 +18,7 @@ #define VMWARE_UTILS_H #include "qemu/range.h" - -#ifndef VMW_SHPRN -#define VMW_SHPRN(fmt, ...) do {} while (0) -#endif +#include "vmxnet_debug.h" /* * Shared memory access functions with byte swap support -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-12-11 3:13 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-05 8:54 [Qemu-devel] [PATCH v2 0/3] fix debug macro pattern for vmxnet3 Miao Yan 2015-12-05 8:55 ` [Qemu-devel] [PATCH v2 1/3] net/vmxnet3.c: fix a build error when enabling debug output Miao Yan 2015-12-05 16:32 ` Eric Blake 2015-12-05 8:55 ` [Qemu-devel] [PATCH v2 2/3] net/vmxnet3: fix debug macro pattern for vmxnet3 Miao Yan 2015-12-07 2:47 ` Jason Wang 2015-12-07 22:04 ` Eric Blake 2015-12-08 2:53 ` Miao Yan 2015-12-11 3:12 ` Jason Wang 2015-12-05 8:55 ` [Qemu-devel] [PATCH v2 3/3] net/vmxnet3: remove redundant VMW_SHPRN(...) definition Miao Yan
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).