* [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
* [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
* [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
* 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
* 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
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).