clang-cl warnings in 'Curl_log_cf_debug()'

[gvanem]

Simply adding this patch:

--- a/lib/curl_log.h 2023-01-19 14:43:40
+++ b/lib/curl_log.h 2023-03-10 13:45:46
@@ -81,7 +81,7 @@
 #endif

 void Curl_log_cf_debug(struct Curl_easy *data, struct Curl_cfilter *cf,
-#if defined(__GNUC__) && !defined(printf) &&                    \
+#if (defined(__GNUC__) || defined(__clang__)) && !defined(printf) && \
   defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L) && \
   !defined(__MINGW32__)
                        const char *fmt, ...)

and using clang-cl to compile with -DDEBUGBUILD, causes these warnings in cf-socket.c and connect.c:

cf-socket.c(978,68): warning: format specifies type 'int' but the argument has type 'curl_socket_t' (aka 'unsigned long long') [-Wformat]
  DEBUGF(LOG_CF(data, cf, "cf_socket_open() -> %d, fd=%d", result, ctx->sock));
                                                      ~~           ^~~~~~~~~
                                                      %llu

cf-socket.c(1585,10): warning: format specifies type 'int' but the argument has type 'curl_socket_t' (aka 'unsigned long long') [-Wformat]
         ctx->sock, ctx->l_ip, ctx->l_port, ctx->r_ip, ctx->r_port));
         ^~~~~~~~~

cf-socket.c(1638,21): warning: format specifies type 'int' but the argument has type 'curl_socket_t' (aka 'unsigned long long') [-Wformat]
                    ctx->sock, ctx->l_ip, ctx->l_port));
                    ^~~~~~~~~

cf-socket.c(1642,38): warning: format specifies type 'int' but the argument has type 'curl_socket_t' (aka 'unsigned long long') [-Wformat]
                    "(unconnected)", ctx->sock));
                                     ^~~~~~~~~
4 warnings generated.

connect.c(667,39): warning: format specifies type 'long' but the argument has type 'timediff_t' (aka 'long long') [-Wformat]
                        baller->name, baller->timeoutms));
                                      ^~~~~~~~~~~~~~~~~

connect.c(805,39): warning: format specifies type 'long' but the argument has type 'timediff_t' (aka 'long long') [-Wformat]
                ctx->baller[0]->name, ctx->baller[0]->timeoutms));
                                      ^~~~~~~~~~~~~~~~~~~~~~~~~

connect.c(816,41): warning: format specifies type 'long' but the argument has type 'timediff_t' (aka 'long long') [-Wformat]
                  ctx->baller[1]->name, ctx->baller[1]->timeoutms));
                                        ^~~~~~~~~~~~~~~~~~~~~~~~~
3 warnings generated.

(edited). I assume the same would occur with gcc on Windows (haven't tested).
Could we use some other format to account for the various curl_socket_t and timediff_t definitions?

[bagder]

We probably need to introduce an (annoying) define for printfing the socket type, as it will be either 32 bit or 64 bit depending on platform. The timediff can probably be output with %lld / "%qd ?

[gvanem]

Using %qd for timediff_t gives no warnings. But what about warnings on other platforms?
Something similar to %zd would suffice AFAICS.

[bagder]

z is crafted for size_t, it seems a little too much to have to make a new flag for every type we ever output, especially for these debug-only outputs.

The q in %qd is for for long long (equal to ll really) which means 64 bit and since timediff_t is 64 bit on all platforms (I hope), I figure that should be good enough.

[vszakats]

With !defined(__MINGW32__) deleted, llvm/clang shows the same as above, and gcc the ones below (it's verbose, sorry):

--- a/lib/curl_log.h
+++ b/lib/curl_log.h
@@ -82,8 +82,7 @@ void Curl_debug(struct Curl_easy *data, curl_infotype type,
 
 void Curl_log_cf_debug(struct Curl_easy *data, struct Curl_cfilter *cf,
 #if defined(__GNUC__) && !defined(printf) &&                    \
-  defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L) && \
-  !defined(__MINGW32__)
+  defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L)
                        const char *fmt, ...)
                        __attribute__((format(printf, 3, 4)));
 #else

gcc:

cf-socket.c:978:27: warning: format '%d' expects argument of type 'int', but argument 5 has type 'curl_socket_t' {aka 'long long unsigned int'} [-Wformat=]
  978 |   DEBUGF(LOG_CF(data, cf, "cf_socket_open() -> %d, fd=%d", result, ctx->sock));
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~          ~~~~~~~~~
      |                                                                       |
      |                                                                       curl_socket_t {aka long long unsigned int}
curl_setup_once.h:285:19: note: in definition of macro 'DEBUGF'
  285 | #define DEBUGF(x) x
      |                   ^
cf-socket.c:978:56: note: format string is defined here
  978 |   DEBUGF(LOG_CF(data, cf, "cf_socket_open() -> %d, fd=%d", result, ctx->sock));
      |                                                       ~^
      |                                                        |
      |                                                        int
      |                                                       %lld
cf-socket.c: In function 'cf_socket_send':
cf-socket.c:1303:27: warning: unknown conversion type character 'z' in format [-Wformat=]
 1303 |   DEBUGF(LOG_CF(data, cf, "send(len=%zu) -> %d, err=%d",
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
curl_setup_once.h:285:19: note: in definition of macro 'DEBUGF'
  285 | #define DEBUGF(x) x
      |                   ^
cf-socket.c:1303:38: note: format string is defined here
 1303 |   DEBUGF(LOG_CF(data, cf, "send(len=%zu) -> %d, err=%d",
      |                                      ^
cf-socket.c:1303:27: warning: format '%d' expects argument of type 'int', but argument 4 has type 'size_t' {aka 'long long unsigned int'} [-Wformat=]
 1303 |   DEBUGF(LOG_CF(data, cf, "send(len=%zu) -> %d, err=%d",
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1304 |                 len, (int)nwritten, *err));
      |                 ~~~        
      |                 |
      |                 size_t {aka long long unsigned int}
curl_setup_once.h:285:19: note: in definition of macro 'DEBUGF'
  285 | #define DEBUGF(x) x
      |                   ^
cf-socket.c:1303:46: note: format string is defined here
 1303 |   DEBUGF(LOG_CF(data, cf, "send(len=%zu) -> %d, err=%d",
      |                                             ~^
      |                                              |
      |                                              int
      |                                             %lld
cf-socket.c:1303:27: warning: too many arguments for format [-Wformat-extra-args]
 1303 |   DEBUGF(LOG_CF(data, cf, "send(len=%zu) -> %d, err=%d",
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
curl_setup_once.h:285:19: note: in definition of macro 'DEBUGF'
  285 | #define DEBUGF(x) x
      |                   ^
cf-socket.c: In function 'cf_socket_recv':
cf-socket.c:1359:27: warning: unknown conversion type character 'z' in format [-Wformat=]
 1359 |   DEBUGF(LOG_CF(data, cf, "recv(len=%zu) -> %d, err=%d", len, (int)nread,
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
curl_setup_once.h:285:19: note: in definition of macro 'DEBUGF'
  285 | #define DEBUGF(x) x
      |                   ^
cf-socket.c:1359:38: note: format string is defined here
 1359 |   DEBUGF(LOG_CF(data, cf, "recv(len=%zu) -> %d, err=%d", len, (int)nread,
      |                                      ^
cf-socket.c:1359:27: warning: format '%d' expects argument of type 'int', but argument 4 has type 'size_t' {aka 'long long unsigned int'} [-Wformat=]
 1359 |   DEBUGF(LOG_CF(data, cf, "recv(len=%zu) -> %d, err=%d", len, (int)nread,
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~  ~~~
      |                                                          |
      |                                                          size_t {aka long long unsigned int}
curl_setup_once.h:285:19: note: in definition of macro 'DEBUGF'
  285 | #define DEBUGF(x) x
      |                   ^
cf-socket.c:1359:46: note: format string is defined here
 1359 |   DEBUGF(LOG_CF(data, cf, "recv(len=%zu) -> %d, err=%d", len, (int)nread,
      |                                             ~^
      |                                              |
      |                                              int
      |                                             %lld
cf-socket.c:1359:27: warning: too many arguments for format [-Wformat-extra-args]
 1359 |   DEBUGF(LOG_CF(data, cf, "recv(len=%zu) -> %d, err=%d", len, (int)nread,
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
curl_setup_once.h:285:19: note: in definition of macro 'DEBUGF'
  285 | #define DEBUGF(x) x
      |                   ^
cf-socket.c: In function 'cf_udp_setup_quic':
cf-socket.c:1583:27: warning: format '%d' expects argument of type 'int', but argument 5 has type 'curl_socket_t' {aka 'long long unsigned int'} [-Wformat=]
 1583 |   DEBUGF(LOG_CF(data, cf, "%s socket %d connected: [%s:%d] -> [%s:%d]",
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1584 |          (ctx->transport == TRNSPRT_QUIC)? "QUIC" : "UDP",
 1585 |          ctx->sock, ctx->l_ip, ctx->l_port, ctx->r_ip, ctx->r_port));
      |          ~~~~~~~~~         
      |             |
      |             curl_socket_t {aka long long unsigned int}
curl_setup_once.h:285:19: note: in definition of macro 'DEBUGF'
  285 | #define DEBUGF(x) x
      |                   ^
cf-socket.c:1583:39: note: format string is defined here
 1583 |   DEBUGF(LOG_CF(data, cf, "%s socket %d connected: [%s:%d] -> [%s:%d]",
      |                                      ~^
      |                                       |
      |                                       int
      |                                      %lld
cf-socket.c: In function 'cf_udp_connect':
cf-socket.c:1637:31: warning: format '%d' expects argument of type 'int', but argument 4 has type 'curl_socket_t' {aka 'long long unsigned int'} [-Wformat=]
 1637 |       DEBUGF(LOG_CF(data, cf, "cf_udp_connect(), opened socket=%d (%s:%d)",
      |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1638 |                     ctx->sock, ctx->l_ip, ctx->l_port));
      |                     ~~~~~~~~~  
      |                        |
      |                        curl_socket_t {aka long long unsigned int}
curl_setup_once.h:285:19: note: in definition of macro 'DEBUGF'
  285 | #define DEBUGF(x) x
      |                   ^
cf-socket.c:1637:65: note: format string is defined here
 1637 |       DEBUGF(LOG_CF(data, cf, "cf_udp_connect(), opened socket=%d (%s:%d)",
      |                                                                ~^
      |                                                                 |
      |                                                                 int
      |                                                                %lld
cf-socket.c:1641:31: warning: format '%d' expects argument of type 'int', but argument 4 has type 'curl_socket_t' {aka 'long long unsigned int'} [-Wformat=]
 1641 |       DEBUGF(LOG_CF(data, cf, "cf_udp_connect(), opened socket=%d "
      |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1642 |                     "(unconnected)", ctx->sock));
      |                                      ~~~~~~~~~
      |                                         |
      |                                         curl_socket_t {aka long long unsigned int}
curl_setup_once.h:285:19: note: in definition of macro 'DEBUGF'
  285 | #define DEBUGF(x) x
      |                   ^
cf-socket.c:1641:65: note: format string is defined here
 1641 |       DEBUGF(LOG_CF(data, cf, "cf_udp_connect(), opened socket=%d "
      |                                                                ~^
      |                                                                 |
      |                                                                 int
      |                                                                %lld
connect.c: In function 'is_connected':
connect.c:666:35: warning: format '%ld' expects argument of type 'long int', but argument 5 has type 'timediff_t' {aka 'long long int'} [-Wformat=]
  666 |           DEBUGF(LOG_CF(data, cf, "%s starting (timeout=%ldms)",
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  667 |                         baller->name, baller->timeoutms));
      |                                       ~~~~~~~~~~~~~~~~~
      |                                             |
      |                                             timediff_t {aka long long int}
curl_setup_once.h:285:19: note: in definition of macro 'DEBUGF'
  285 | #define DEBUGF(x) x
      |                   ^
connect.c:666:59: note: format string is defined here
  666 |           DEBUGF(LOG_CF(data, cf, "%s starting (timeout=%ldms)",
      |                                                         ~~^
      |                                                           |
      |                                                           long int
      |                                                         %lld
connect.c: In function 'start_connect':
connect.c:804:27: warning: format '%ld' expects argument of type 'long int', but argument 5 has type 'timediff_t' {aka 'long long int'} [-Wformat=]
  804 |   DEBUGF(LOG_CF(data, cf, "created %s (timeout %ldms)",
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
  805 |                 ctx->baller[0]->name, ctx->baller[0]->timeoutms));
      |                                       ~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                     |
      |                                                     timediff_t {aka long long int}
curl_setup_once.h:285:19: note: in definition of macro 'DEBUGF'
  285 | #define DEBUGF(x) x
      |                   ^
connect.c:804:50: note: format string is defined here
  804 |   DEBUGF(LOG_CF(data, cf, "created %s (timeout %ldms)",
      |                                                ~~^
      |                                                  |
      |                                                  long int
      |                                                %lld
connect.c:815:29: warning: format '%ld' expects argument of type 'long int', but argument 5 has type 'timediff_t' {aka 'long long int'} [-Wformat=]
  815 |     DEBUGF(LOG_CF(data, cf, "created %s (timeout %ldms)",
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
  816 |                   ctx->baller[1]->name, ctx->baller[1]->timeoutms));
      |                                         ~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                       |
      |                                                       timediff_t {aka long long int}
curl_setup_once.h:285:19: note: in definition of macro 'DEBUGF'
  285 | #define DEBUGF(x) x
      |                   ^
connect.c:815:52: note: format string is defined here
  815 |     DEBUGF(LOG_CF(data, cf, "created %s (timeout %ldms)",
      |                                                  ~~^
      |                                                    |
      |                                                    long int
      |                                                  %lld
In function 'is_connected',
    inlined from 'cf_he_connect' at connect.c:897:16:
connect.c:688:29: warning: '%s' directive argument is null [-Wformat-overflow=]
  688 |     DEBUGF(LOG_CF(data, cf, "%s assess started=%d, result=%d",
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
curl_setup_once.h:285:19: note: in definition of macro 'DEBUGF'
  285 | #define DEBUGF(x) x
      |                   ^
connect.c: In function 'cf_he_connect':
connect.c:688:30: note: format string is defined here
  688 |     DEBUGF(LOG_CF(data, cf, "%s assess started=%d, result=%d",
      |                              ^~

[gvanem]

Works fine now; no warnings from clang. So could the curl_log.h now be patched to:

--- a/lib/curl_log.h 2023-01-19 14:43:40
+++ b/lib/curl_log.h 2023-03-30 07:48:49
@@ -81,7 +81,7 @@
 #endif

 void Curl_log_cf_debug(struct Curl_easy *data, struct Curl_cfilter *cf,
-#if defined(__GNUC__) && !defined(printf) &&                    \
+#if (defined(__GNUC__) || defined(__clang__)) && !defined(printf) && \
   defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L) && \
   !defined(__MINGW32__)
                        const char *fmt, ...)

[vszakats]

We can probably also delete !defined(__MINGW32__) now.