Eric Blake <ebl...@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 <yanmiaob...@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.
