2015-12-04 14:58 GMT+08:00 阎淼 <yanmiaob...@gmail.com>:
> 2015-12-03 23:59 GMT+08:00 Eric Blake <ebl...@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 <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:
>
>
> 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
>>
