]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
sectransp: fix clang compiler warnings, stop silencing them
authorViktor Szakats <commit@vsz.me>
Sat, 6 Jul 2024 12:59:57 +0000 (14:59 +0200)
committerViktor Szakats <commit@vsz.me>
Sat, 13 Jul 2024 10:00:13 +0000 (12:00 +0200)
Fix `-Wpointer-bool-conversion` warnings with the method suggested by
both Apple clang and mainline llvm. This was already tried and dropped
in #1705 (in year 2017), but the issue reported there no longer
replicates.

Verified with Apple clang 14, llvm 15, llvm 18 and gcc 11, 14 that the
generated objects are bit by bit identical before and after this patch.

Also:
- stop silencing `-Wtautological-pointer-compare`. This warning don't
  seem to be appearing anymore (with or without this patch), at least
  with the tested compilers and SDKs (clang 13.1.6-16.0.0beta, llvm 15,
  18, gcc 11, 14) and minimum macOS target of 10.8. Older targets fail
  to build curl with SecureTransport.

- silence `-Wunreachable-code` for clang only. Previously I applied it
  also to GCC, by mistake.
  Ref: https://github.com/curl/curl/pull/12331/commits/8d7172d20a48ebc6c1b1d94a76e2c5fb19dd9bfa

Apple clang `-Wpointer-bool-conversion`:
```
curl/lib/vtls/sectransp.c:1103:6: error: address of function 'SSLCreateContext' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
  if(SSLCreateContext) {  /* use the newer API if available */
  ~~ ^~~~~~~~~~~~~~~~
curl/lib/vtls/sectransp.c:1103:6: note: prefix with the address-of operator to silence this warning
  if(SSLCreateContext) {  /* use the newer API if available */
     ^
     &
```
Ref: https://github.com/curl/curl/actions/runs/9819538439/job/27113201384#step:8:382

llvm `-Wpointer-bool-conversion`:
```
curl/lib/vtls/sectransp.c:2663:8: error: address of function 'SSLCreateContext' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
    if(SSLCreateContext)
    ~~ ^~~~~~~~~~~~~~~~
curl/lib/vtls/sectransp.c:2663:8: note: prefix with the address-of operator to silence this warning
    if(SSLCreateContext)
       ^
       &
```
Ref: https://github.com/curl/curl/actions/runs/9819538439/job/27113200291#step:8:417

gcc still needs `-Waddress` suppressed to avoid these:
```
curl/lib/vtls/n/sectransp.c: In function 'getsubject':
curl/lib/vtls/n/sectransp.c:379:6: warning: the address of 'SecCertificateCopyLongDescription' will always evaluate as 'true' [-Waddress]
  379 |   if(&SecCertificateCopyLongDescription)
      |      ^
[...]
```

Follow-up to 59cadacfcc1d39472245979cdbd614c7a9af6f0d #14128
Follow-up to af271ce9b9717ba289417e9cbb7f278c2a12f959 #1722
Follow-up to 2b7ce3f56dfede107113c6de7d0ca457109d3eda #1706
Cherry-picked from #14097
Closes #14162

lib/vtls/sectransp.c

index 89993b17199eb063a1499fe69f5625a2ed3e18f8..c2803e8bfc751662ced6509000df012b5ffe34be 100644 (file)
 
 #ifdef __clang__
 #pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wpointer-bool-conversion"
-#pragma clang diagnostic ignored "-Wtautological-pointer-compare"
+#pragma clang diagnostic ignored "-Wunreachable-code"
 #endif /* __clang__ */
 
 #ifdef __GNUC__
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Waddress"
-#pragma GCC diagnostic ignored "-Wunreachable-code"
 #endif
 
 #include <limits.h>
@@ -377,14 +375,14 @@ CF_INLINE CFStringRef getsubject(SecCertificateRef cert)
 #else
 #if CURL_BUILD_MAC_10_7
   /* Lion & later: Get the long description if we can. */
-  if(SecCertificateCopyLongDescription)
+  if(&SecCertificateCopyLongDescription)
     server_cert_summary =
       SecCertificateCopyLongDescription(NULL, cert, NULL);
   else
 #endif /* CURL_BUILD_MAC_10_7 */
 #if CURL_BUILD_MAC_10_6
   /* Snow Leopard: Get the certificate summary. */
-  if(SecCertificateCopySubjectSummary)
+  if(&SecCertificateCopySubjectSummary)
     server_cert_summary = SecCertificateCopySubjectSummary(cert);
   else
 #endif /* CURL_BUILD_MAC_10_6 */
@@ -497,7 +495,7 @@ static OSStatus CopyIdentityWithLabel(char *label,
   /* SecItemCopyMatching() was introduced in iOS and Snow Leopard.
      kSecClassIdentity was introduced in Lion. If both exist, let's use them
      to find the certificate. */
-  if(SecItemCopyMatching && kSecClassIdentity) {
+  if(&SecItemCopyMatching && kSecClassIdentity) {
     CFTypeRef keys[5];
     CFTypeRef values[5];
     CFDictionaryRef query_dict;
@@ -793,7 +791,7 @@ static CURLcode set_ssl_version_min_max(struct Curl_cfilter *cf,
   }
 
 #if CURL_BUILD_MAC_10_8 || CURL_BUILD_IOS
-  if(SSLSetProtocolVersionMax) {
+  if(&SSLSetProtocolVersionMax) {
     SSLProtocol darwin_ver_min = kTLSProtocol1;
     SSLProtocol darwin_ver_max = kTLSProtocol1;
     CURLcode result = sectransp_version_from_curl(&darwin_ver_min,
@@ -1101,7 +1099,7 @@ static CURLcode sectransp_connect_step1(struct Curl_cfilter *cf,
 #endif /* CURL_BUILD_MAC */
 
 #if CURL_BUILD_MAC_10_8 || CURL_BUILD_IOS
-  if(SSLCreateContext) {  /* use the newer API if available */
+  if(&SSLCreateContext) {  /* use the newer API if available */
     if(backend->ssl_ctx)
       CFRelease(backend->ssl_ctx);
     backend->ssl_ctx = SSLCreateContext(NULL, kSSLClientSide, kSSLStreamType);
@@ -1135,7 +1133,7 @@ static CURLcode sectransp_connect_step1(struct Curl_cfilter *cf,
 
   /* check to see if we have been told to use an explicit SSL/TLS version */
 #if CURL_BUILD_MAC_10_8 || CURL_BUILD_IOS
-  if(SSLSetProtocolVersionMax) {
+  if(&SSLSetProtocolVersionMax) {
     switch(conn_config->version) {
     case CURL_SSLVERSION_TLSv1:
       (void)SSLSetProtocolVersionMin(backend->ssl_ctx, kTLSProtocol1);
@@ -1386,9 +1384,9 @@ static CURLcode sectransp_connect_step1(struct Curl_cfilter *cf,
   Darwin 15.x.x is El Capitan (10.11)
   */
 #if CURL_BUILD_MAC
-  if(SSLSetSessionOption && darwinver_maj >= 13) {
+  if(&SSLSetSessionOption && darwinver_maj >= 13) {
 #else
-  if(SSLSetSessionOption) {
+  if(&SSLSetSessionOption) {
 #endif /* CURL_BUILD_MAC */
     bool break_on_auth = !conn_config->verifypeer ||
       ssl_cafile || ssl_cablob;
@@ -1469,7 +1467,7 @@ static CURLcode sectransp_connect_step1(struct Curl_cfilter *cf,
 #if CURL_BUILD_MAC_10_9 || CURL_BUILD_IOS_7
   /* We want to enable 1/n-1 when using a CBC cipher unless the user
      specifically does not want us doing that: */
-  if(SSLSetSessionOption) {
+  if(&SSLSetSessionOption) {
     SSLSetSessionOption(backend->ssl_ctx, kSSLSessionOptionSendOneByteRecord,
                         !ssl_config->enable_beast);
     SSLSetSessionOption(backend->ssl_ctx, kSSLSessionOptionFalseStart,
@@ -2366,7 +2364,7 @@ static CURLcode collect_server_cert(struct Curl_cfilter *cf,
      private API and does not work as expected. So we have to look for
      a different symbol to make sure this code is only executed under
      Lion or later. */
-  if(SecTrustCopyPublicKey) {
+  if(&SecTrustCopyPublicKey) {
 #pragma unused(server_certs)
     err = SSLCopyPeerTrust(backend->ssl_ctx, &trust);
     /* For some reason, SSLCopyPeerTrust() can return noErr and yet return
@@ -2661,7 +2659,7 @@ static void sectransp_close(struct Curl_cfilter *cf, struct Curl_easy *data)
   if(backend->ssl_ctx) {
     CURL_TRC_CF(data, cf, "close");
 #if CURL_BUILD_MAC_10_8 || CURL_BUILD_IOS
-    if(SSLCreateContext)
+    if(&SSLCreateContext)
       CFRelease(backend->ssl_ctx);
 #if CURL_SUPPORT_MAC_10_8
     else
@@ -2736,7 +2734,7 @@ static CURLcode sectransp_sha256sum(const unsigned char *tmp, /* input */
 static bool sectransp_false_start(void)
 {
 #if CURL_BUILD_MAC_10_9 || CURL_BUILD_IOS_7
-  if(SSLSetSessionOption)
+  if(&SSLSetSessionOption)
     return TRUE;
 #endif
   return FALSE;