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

[Eric Blake <eblake@redhat.com>]

[-- 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 --]