Skip to content

mbedtls: fix -Wnull-dereference and -Wredundant-decls #12720

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

mbedtls: fix -Wnull-dereference and -Wredundant-decls #12720

wants to merge 1 commit into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Jan 16, 2024 •
edited
Loading

  • Silence warning in mbedTLS v3.5.1 public headers:

    ./mbedtls/_x64-linux-musl/usr/include/psa/crypto_extra.h:489:14: warning: redundant redeclaration of 'psa_set_key_domain_parameters' [-Wredundant-decls]
./mbedtls/_x64-linux-musl/usr/include/psa/crypto_struct.h:354:14: note: previous declaration of 'psa_set_key_domain_parameters' was here

    Ref: libssh2/libssh2@ecec68a
    Ref: mbedtls: improve disabling -Wredundant-decls libssh2/libssh2#1226

  • Fix compiler warnings seen with gcc 9.2.0 + cmake unity:

    ./curl/lib/vtls/mbedtls.c: In function 'mbedtls_bio_cf_read':
./curl/lib/vtls/mbedtls.c:189:11: warning: null pointer dereference [-Wnull-dereference]
  189 |   nread = Curl_conn_cf_recv(cf->next, data, (char *)buf, blen, &result);
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./curl/lib/vtls/mbedtls.c: In function 'mbedtls_bio_cf_write':
./curl/lib/vtls/mbedtls.c:168:14: warning: null pointer dereference [-Wnull-dereference]
  168 |   nwritten = Curl_conn_cf_send(cf->next, data, (char *)buf, blen, &result);
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  • delete stray #else.

Closes #12720

vszakats added a commit to curl/curl-for-win that referenced this pull request Jan 16, 2024
@vszakats
curl.patch: add patch for mbedtls [ci skip]
d2231da 
Ref: curl/curl#12720
@vszakats

This comment was marked as outdated.

Sign in to view
@vszakats vszakats requested a review from icing January 17, 2024 08:42
@vszakats
mbedtls: fix -Wnull-dereference and -Wredundant-decls
ad02c64 
- Silence warning in mbedTLS v3.5.1 public headers:
  ```
  ./mbedtls/_x64-linux-musl/usr/include/psa/crypto_extra.h:489:14: warning: redundant redeclaration of 'psa_set_key_domain_parameters' [-Wredundant-decls]
  ./mbedtls/_x64-linux-musl/usr/include/psa/crypto_struct.h:354:14: note: previous declaration of 'psa_set_key_domain_parameters' was here
  ```
  Ref: libssh2/libssh2@ecec68a
  Ref: libssh2/libssh2#1226

- Fix compiler warnings seen with gcc 9.2.0 + cmake unity:
  ```
  ./curl/lib/vtls/mbedtls.c: In function 'mbedtls_bio_cf_read':
  ./curl/lib/vtls/mbedtls.c:189:11: warning: null pointer dereference [-Wnull-dereference]
    189 |   nread = Curl_conn_cf_recv(cf->next, data, (char *)buf, blen, &result);
        |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ./curl/lib/vtls/mbedtls.c: In function 'mbedtls_bio_cf_write':
  ./curl/lib/vtls/mbedtls.c:168:14: warning: null pointer dereference [-Wnull-dereference]
    168 |   nwritten = Curl_conn_cf_send(cf->next, data, (char *)buf, blen, &result);
        |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ```

- delete stray `#else`.

Closes curl#12720
@vszakats vszakats closed this in 434db99 Jan 18, 2024
@vszakats vszakats deleted the mbedtls-fix branch January 18, 2024 07:10
@vszakats vszakats mentioned this pull request Sep 3, 2024
Closed
5 tasks
vszakats added a commit to vszakats/curl that referenced this pull request Sep 19, 2024
@vszakats
cmake, Makefile.mk: use -isystem for dep headers, silence BearSSL…
445fb81 
… issues

Patch started out for working around compiler warnings in BearSSL latest
tarball release v0.6 (2018-08-14) and Apple clang 14 with CMake.

Then turned into patching CMake and `Makefile.mk` builds to use
`-isystem` instead `-I` when adding header directories for
dependencies. This avoids compiler warnings in dependency headers,
syncing behaviour with autotools.

Also:
- `Makefile.mk`: add support for BearSSL.
- delete warning suppression for mbedTLS headers. No longer necessary
  after this patch.
  Follow-up to 434db99 curl#12720

Silenced BearSSL warnings:
```
In file included from curl/lib/vtls/bearssl.c:28:
In file included from bearssl/inc/bearssl.h:127:
bearssl/inc/bearssl_hash.h:727:5: warning: 'BR_DOXYGEN_IGNORE' is not defined, evaluates to 0 [-Wundef]
    ^
bearssl/inc/bearssl_hash.h:745:5: warning: 'BR_DOXYGEN_IGNORE' is not defined, evaluates to 0 [-Wundef]
    ^
In file included from curl/lib/vtls/bearssl.c:28:
In file included from bearssl/inc/bearssl.h:136:
bearssl/inc/bearssl_ssl.h:1253:20: warning: implicit conversion loses integer precision: 'unsigned int' to 'uint16_t' (aka 'unsigned short') [-Wimplicit-int-conversion]
        cc->version_min = version_min;
                        ~ ^~~~~~~~~~~
bearssl/inc/bearssl_ssl.h:1254:20: warning: implicit conversion loses integer precision: 'unsigned int' to 'uint16_t' (aka 'unsigned short') [-Wimplicit-int-conversion]
        cc->version_max = version_max;
                        ~ ^~~~~~~~~~~
bearssl/inc/bearssl_ssl.h:1327:28: warning: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'uint16_t' (aka 'unsigned short') [-Wimplicit-int-conversion]
        ctx->protocol_names_num = num;
                                ~ ^~~
5 warnings generated.
```

(These warnings were fixed in BearSSL Git master in 2019 via
2893441f2efd4603ddd6d7f49011bdda096a4a87 and
ecdf89770ee82dfea6186fb4369cff3d06cd852e.)

Also these two cases, which are caused by an unidentified component
(outside curl) cranking up MSVC warnings in external headers to `/W4`
when ZLIB is deselected:
curl#14859 (comment)

mbedTLS 3.6.1:
```
C:\vcpkg\installed\x64-windows\include\psa\crypto_struct.h(254,13): error C2220: the following warning is treated as an error [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
  (compiling source file 'CMakeFiles/libcurl_object.dir/Unity/unity_0_c.c')
C:\vcpkg\installed\x64-windows\include\psa\crypto_struct.h(254,13): warning C4200: nonstandard extension used: zero-sized array in struct/union [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
  (compiling source file 'CMakeFiles/libcurl_object.dir/Unity/unity_0_c.c')
```
Ref: https://github.com/curl/curl/actions/runs/10842694205/job/30107466989?pr=14859#step:10:29

nghttp3 1.5.0:
```
C:\vcpkg\installed\x64-windows\include\nghttp3\nghttp3.h(2678,1): error C2220: the following warning is treated as an error [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
  (compiling source file 'CMakeFiles/libcurl_object.dir/Unity/unity_0_c.c')
  C:\vcpkg\installed\x64-windows\include\nghttp3\nghttp3.h(2678,1): warning C4324: 'nghttp3_pri': structure was padded due to alignment specifier [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
  (compiling source file 'CMakeFiles/libcurl_object.dir/Unity/unity_0_c.c')
```
Ref: https://github.com/curl/curl/actions/runs/10871875297/job/30166233862?pr=14859#step:10:28

Closes curl#14763
pps83 pushed a commit to pps83/curl that referenced this pull request Apr 26, 2025
@vszakats @pps83
cmake, Makefile.mk: use -isystem for dep headers, silence BearSSL…
6b1f13d 
… issues

Patch started out for working around compiler warnings in BearSSL latest
tarball release v0.6 (2018-08-14) and Apple clang 14 with CMake.

Then turned into patching CMake and `Makefile.mk` builds to use
`-isystem` instead `-I` when adding header directories for
dependencies. This avoids compiler warnings in dependency headers,
syncing behaviour with autotools.

Also:
- `Makefile.mk`: add support for BearSSL.
- delete warning suppression for mbedTLS headers. No longer necessary
  after this patch.
  Follow-up to 434db99 curl#12720

Silenced BearSSL warnings:
```
In file included from curl/lib/vtls/bearssl.c:28:
In file included from bearssl/inc/bearssl.h:127:
bearssl/inc/bearssl_hash.h:727:5: warning: 'BR_DOXYGEN_IGNORE' is not defined, evaluates to 0 [-Wundef]
    ^
bearssl/inc/bearssl_hash.h:745:5: warning: 'BR_DOXYGEN_IGNORE' is not defined, evaluates to 0 [-Wundef]
    ^
In file included from curl/lib/vtls/bearssl.c:28:
In file included from bearssl/inc/bearssl.h:136:
bearssl/inc/bearssl_ssl.h:1253:20: warning: implicit conversion loses integer precision: 'unsigned int' to 'uint16_t' (aka 'unsigned short') [-Wimplicit-int-conversion]
        cc->version_min = version_min;
                        ~ ^~~~~~~~~~~
bearssl/inc/bearssl_ssl.h:1254:20: warning: implicit conversion loses integer precision: 'unsigned int' to 'uint16_t' (aka 'unsigned short') [-Wimplicit-int-conversion]
        cc->version_max = version_max;
                        ~ ^~~~~~~~~~~
bearssl/inc/bearssl_ssl.h:1327:28: warning: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'uint16_t' (aka 'unsigned short') [-Wimplicit-int-conversion]
        ctx->protocol_names_num = num;
                                ~ ^~~
5 warnings generated.
```

(These warnings were fixed in BearSSL Git master in 2019 via
2893441f2efd4603ddd6d7f49011bdda096a4a87 and
ecdf89770ee82dfea6186fb4369cff3d06cd852e.)

Also these two cases, which are caused by an unidentified component
(outside curl) cranking up MSVC warnings in external headers to `/W4`
when ZLIB is deselected:
curl#14859 (comment)

mbedTLS 3.6.1:
```
C:\vcpkg\installed\x64-windows\include\psa\crypto_struct.h(254,13): error C2220: the following warning is treated as an error [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
  (compiling source file 'CMakeFiles/libcurl_object.dir/Unity/unity_0_c.c')
C:\vcpkg\installed\x64-windows\include\psa\crypto_struct.h(254,13): warning C4200: nonstandard extension used: zero-sized array in struct/union [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
  (compiling source file 'CMakeFiles/libcurl_object.dir/Unity/unity_0_c.c')
```
Ref: https://github.com/curl/curl/actions/runs/10842694205/job/30107466989?pr=14859#step:10:29

nghttp3 1.5.0:
```
C:\vcpkg\installed\x64-windows\include\nghttp3\nghttp3.h(2678,1): error C2220: the following warning is treated as an error [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
  (compiling source file 'CMakeFiles/libcurl_object.dir/Unity/unity_0_c.c')
  C:\vcpkg\installed\x64-windows\include\nghttp3\nghttp3.h(2678,1): warning C4324: 'nghttp3_pri': structure was padded due to alignment specifier [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
  (compiling source file 'CMakeFiles/libcurl_object.dir/Unity/unity_0_c.c')
```
Ref: https://github.com/curl/curl/actions/runs/10871875297/job/30166233862?pr=14859#step:10:28

Closes curl#14763
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@icing icing Awaiting requested review from icing

Assignees
No one assigned
Labels
build TLS
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@vszakats