[Spice-devel] [usbredir][PATCH] usbredirhost: Fix -Wformat warning

[Hans de Goede]

Hi,

On 03-11-15 01:52, Fabiano Fidêncio wrote:
> On Tue, Nov 3, 2015 at 1:51 AM, Fabiano Fidêncio <fidencio at redhat.com> wrote:
>> Cast uint64_t to long unsigned on printfs in order to avoid warnings
>> like:
>> usbredirhost.c: In function 'usbredirhost_can_write_iso_package':
>> usbredirhost.c:1023:19: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'uint64_t {aka long long unsigned int}' [-Wformat=]
>>               DEBUG("START dropping isoc packets %lu buffer > %lu hi threshold",
>>                     ^
>> usbredirhost.c:181:57: note: in definition of macro 'DEBUG'
>>   #define DEBUG(...)   va_log(host, usbredirparser_debug, __VA_ARGS__)
>>                                                           ^
>> usbredirhost.c:1023:19: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'uint64_t {aka long long unsigned int}' [-Wformat=]
>>               DEBUG("START dropping isoc packets %lu buffer > %lu hi threshold",
>>                     ^
>> usbredirhost.c:181:57: note: in definition of macro 'DEBUG'
>>   #define DEBUG(...)   va_log(host, usbredirparser_debug, __VA_ARGS__)
>>                                                           ^
>> usbredirhost.c:1028:19: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'uint64_t {aka long long unsigned int}' [-Wformat=]
>>               DEBUG("STOP dropping isoc packets %lu buffer < %lu low threshold",
>>                     ^
>> usbredirhost.c:181:57: note: in definition of macro 'DEBUG'
>>   #define DEBUG(...)   va_log(host, usbredirparser_debug, __VA_ARGS__)
>>                                                           ^
>> usbredirhost.c:1028:19: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'uint64_t {aka long long unsigned int}' [-Wformat=]
>>               DEBUG("STOP dropping isoc packets %lu buffer < %lu low threshold",
>>                     ^
>> usbredirhost.c:181:57: note: in definition of macro 'DEBUG'
>>   #define DEBUG(...)   va_log(host, usbredirparser_debug, __VA_ARGS__)
>>                                                           ^
>> usbredirhost.c: In function 'usbredirhost_set_iso_threshold':
>> usbredirhost.c:1162:11: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'uint64_t {aka long long unsigned int}' [-Wformat=]
>>       DEBUG("higher threshold is %lu bytes | lower threshold is %lu bytes",
>>             ^
>> usbredirhost.c:181:57: note: in definition of macro 'DEBUG'
>>   #define DEBUG(...)   va_log(host, usbredirparser_debug, __VA_ARGS__)
>>                                                           ^
>> usbredirhost.c:1162:11: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'uint64_t {aka long long unsigned int}' [-Wformat=]
>>       DEBUG("higher threshold is %lu bytes | lower threshold is %lu bytes",
>>             ^
>> usbredirhost.c:181:57: note: in definition of macro 'DEBUG'
>>   #define DEBUG(...)   va_log(host, usbredirparser_debug, __VA_ARGS__)
>>
>> A better way to solve the problem would be using %zu (C99 only) instead
>> of doing the cast, but then mingw32 complains about:
>> "warning: unknown conversion type character 'z' in format [-Wformat=]"
>>
>> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
>> ---
>>   usbredirhost/usbredirhost.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/usbredirhost/usbredirhost.c b/usbredirhost/usbredirhost.c
>> index adf9c5f..bfccfab 100644
>> --- a/usbredirhost/usbredirhost.c
>> +++ b/usbredirhost/usbredirhost.c
>> @@ -1021,12 +1021,14 @@ static int usbredirhost_can_write_iso_package(struct usbredirhost *host)
>>       if (size >= host->iso_threshold.higher) {
>>           if (!host->iso_threshold.dropping)
>>               DEBUG("START dropping isoc packets %lu buffer > %lu hi threshold",
>> -                  size, host->iso_threshold.higher);
>> +                  (long unsigned) size,
>> +                  (long unsigned) host->iso_threshold.higher);
>>           host->iso_threshold.dropping = true;
>>       } else if (size < host->iso_threshold.lower) {
>>           if (host->iso_threshold.dropping)
>>               DEBUG("STOP dropping isoc packets %lu buffer < %lu low threshold",
>> -                  size, host->iso_threshold.lower);
>> +                  (long unsigned) size,
>> +                  (long unsigned) host->iso_threshold.lower);
>>
>>           host->iso_threshold.dropping = false;
>>       }
>> @@ -1160,7 +1162,8 @@ static void usbredirhost_set_iso_threshold(struct usbredirhost *host,
>>       host->iso_threshold.lower = reference / 2;
>>       host->iso_threshold.higher = reference * 3;
>>       DEBUG("higher threshold is %lu bytes | lower threshold is %lu bytes",
>> -           host->iso_threshold.higher, host->iso_threshold.lower);
>> +           (long unsigned) host->iso_threshold.higher,
>> +           (long unsigned) host->iso_threshold.lower);
>>   }
>>
>>   /* Called from both parser read and packet complete callbacks */
>> --
>> 2.5.0
>>
>
> Adding Hans to the loop ...

Thanks.

The proper fix for this would be to do:

#include <inttypes.h>

And then change the log lines to e.g.:

DEBUG("higher threshold is %" PRIu64 " bytes | lower threshold is %" PRIu64 " bytes",
       host->iso_threshold.higher, host->iso_threshold.lower);

This is the proper way to deal with printf-ing uint64_t variables.

Regards,

Hans