Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clang-cl warnings in 'Curl_log_cf_debug()' #10737

Closed
gvanem opened this issue Mar 10, 2023 · 7 comments
Closed

clang-cl warnings in 'Curl_log_cf_debug()' #10737

gvanem opened this issue Mar 10, 2023 · 7 comments
Labels
build Windows Windows-specific

Comments

@gvanem
Copy link
Contributor

gvanem commented Mar 10, 2023

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
Copy link
Member

bagder commented Mar 10, 2023

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 ?

@bagder bagder added the build label Mar 10, 2023
@gvanem
Copy link
Contributor Author

gvanem commented Mar 10, 2023

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

@bagder
Copy link
Member

bagder commented Mar 10, 2023

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 vszakats added the Windows Windows-specific label Mar 11, 2023
@vszakats
Copy link
Member

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",
      |                              ^~

bagder added a commit that referenced this issue Mar 28, 2023
bagder added a commit that referenced this issue Mar 29, 2023
Introduces CURL_FORMAT_SOCKET_T for outputting socket numbers.

Fixes #10737
Reported-by: Gisle Vanem
Closes #10855
@bagder bagder closed this as completed in 8455013 Mar 29, 2023
@gvanem
Copy link
Contributor Author

gvanem commented Mar 30, 2023

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
Copy link
Member

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

bch pushed a commit to bch/curl that referenced this issue Jul 19, 2023
Introduces CURL_FORMAT_SOCKET_T for outputting socket numbers.

Fixes curl#10737
Reported-by: Gisle Vanem
Closes curl#10855
@cpsauer
Copy link
Contributor

cpsauer commented Sep 29, 2023

I think I'm seeing a few more of these squeaking through in 8.3.0 for 32 bit. Browsing through in each case, the format specifier is hardcoded as %ld rather than CURL_FORMAT_CURL_OFF_T

INFO: From Compiling lib/connect.c:
external/curl/lib/connect.c:650:11: warning: format specifies type 'long' but the argument has type 'timediff_t' (aka 'long long') [-Wformat]
          Curl_timediff(now, data->progress.t_startsingle));
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
INFO: From Compiling lib/url.c:
external/curl/lib/url.c:892:11: warning: format specifies type 'long' but the argument has type 'timediff_t' (aka 'long long') [-Wformat]
          idletime);
          ^~~~~~~~
external/curl/lib/curl_trc.h:120:27: note: expanded from macro 'infof'
         Curl_infof(data, __VA_ARGS__); } while(0)
                          ^~~~~~~~~~~
external/curl/lib/url.c:902:11: warning: format specifies type 'long' but the argument has type 'timediff_t' (aka 'long long') [-Wformat]
          lifetime);
          ^~~~~~~~
external/curl/lib/curl_trc.h:120:27: note: expanded from macro 'infof'
         Curl_infof(data, __VA_ARGS__); } while(0)
                          ^~~~~~~~~~~
external/curl/lib/url.c:3219:11: warning: format specifies type 'long' but the argument has type 'timediff_t' (aka 'long long') [-Wformat]
          Curl_timediff(Curl_now(), data->progress.t_startsingle));
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3 warnings generated.
INFO: From Compiling lib/url.c:
external/curl/lib/url.c:892:11: warning: format specifies type 'long' but the argument has type 'timediff_t' (aka 'long long') [-Wformat]
          idletime);
          ^~~~~~~~
external/curl/lib/curl_trc.h:120:27: note: expanded from macro 'infof'
         Curl_infof(data, __VA_ARGS__); } while(0)
                          ^~~~~~~~~~~
external/curl/lib/url.c:902:11: warning: format specifies type 'long' but the argument has type 'timediff_t' (aka 'long long') [-Wformat]
          lifetime);
          ^~~~~~~~
external/curl/lib/curl_trc.h:120:27: note: expanded from macro 'infof'
         Curl_infof(data, __VA_ARGS__); } while(0)
                          ^~~~~~~~~~~
external/curl/lib/url.c:3219:11: warning: format specifies type 'long' but the argument has type 'timediff_t' (aka 'long long') [-Wformat]
          Curl_timediff(Curl_now(), data->progress.t_startsingle));
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3 warnings generated.
INFO: From Compiling lib/connect.c:
external/curl/lib/connect.c:650:11: warning: format specifies type 'long' but the argument has type 'timediff_t' (aka 'long long') [-Wformat]
          Curl_timediff(now, data->progress.t_startsingle));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Windows Windows-specific
Development

Successfully merging a pull request may close this issue.

4 participants