Re: [Qemu-devel] [PATCH v2 2/3] net/vmxnet3: fix debug macro pattern for vmxnet3

[Jason Wang]

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 <address@hidden>
>>> Reviewed-by: Eric Blake <address@hidden>
>>> ---
>>> 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