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

lib/cf-h1-proxy.c: -Wnull-dereference warnings with gcc 12 / curl 8.7.1 #13237

Closed
vszakats opened this issue Mar 30, 2024 · 3 comments
Closed

Comments

@vszakats
Copy link
Member

vszakats commented Mar 30, 2024

I did this

Built curl-for-win with gcc + mingw-w64.
Got these warnings:

.../curl/lib/cf-h1-proxy.c:1060:17: warning: null pointer dereference [-Wnull-dereference]
 1060 |   cf->connected = FALSE;
      |                 ^
.../curl/lib/cf-h1-proxy.c:1061:8: warning: null pointer dereference [-Wnull-dereference]
 1061 |   if(cf->ctx) {
      |      ~~^~~~~
In function 'tunnel_free',
    inlined from 'cf_h1_proxy_destroy' at .../curl/lib/cf-h1-proxy.c:1053:3:
.../curl/lib/cf-h1-proxy.c:198:27: warning: null pointer dereference [-Wnull-dereference]
  198 |   struct h1_tunnel_state *ts = cf->ctx;
      |                           ^~

Ref: https://github.com/curl/curl-for-win/actions/runs/8492022102/job/23264582018#step:3:10749

I expected the following

No warnings.

curl/libcurl version

curl 8.7.1

operating system

Windows (seems OS-agnostic)

@bagder
Copy link
Member

bagder commented Mar 30, 2024

Grr, this the overcomplaining errors from gcc's static analyzer coming. Quite annoying.

This happens because in:

  CURL_TRC_CF(data, cf, "close");
  cf->connected = FALSE;

... the macro on the first line checks if cf is non-NULL, which then gcc takes as a confirmation it should complain on line two which uses cf without a NULL check.

I find this warning annoying and counter-productive. The macro is fine to use protection because it can be used in places where maybe it can be a NULL pointer passed to it. It does mean that a NULL pointer is accepted in every place the macro is used.

@vszakats vszakats changed the title lib/cf-h1-proxy.c -Wnull-dereference warnings with gcc 13.2.0 / curl 8.7.1 lib/cf-h1-proxy.c -Wnull-dereference warnings with gcc 12.0.0 / curl 8.7.1 Mar 30, 2024
@vszakats
Copy link
Member Author

vszakats commented Mar 30, 2024

I guess a few false positives is the price to pay for detecting real issues.
Also an extra NULL check here in there is probably better than missing
one by accident. But yeah, this is annoying.

The other thing I wondered why this wasn't caught in CI. E.g. in this job (mingw, gcc13, unity): https://ci.appveyor.com/project/curlorg/curl/builds/49500338/job/opyx2nhk46vop8b3?fullLog=true

Then noticed that the affected gcc version was actually 12.2.0, not 13.2.0.
So this may have been fixed, unless I'm missing some other difference.

@vszakats vszakats changed the title lib/cf-h1-proxy.c -Wnull-dereference warnings with gcc 12.0.0 / curl 8.7.1 lib/cf-h1-proxy.c -Wnull-dereference warnings with gcc 12 / curl 8.7.1 Mar 30, 2024
@vszakats vszakats changed the title lib/cf-h1-proxy.c -Wnull-dereference warnings with gcc 12 / curl 8.7.1 lib/cf-h1-proxy.c: -Wnull-dereference warnings with gcc 12 / curl 8.7.1 Apr 1, 2024
@vszakats
Copy link
Member Author

vszakats commented Apr 2, 2024

This remains valid, but I don't want to offend anybody with warnings popping up in tests.

@vszakats vszakats closed this as completed Apr 2, 2024
vszakats added a commit to vszakats/curl that referenced this issue May 7, 2024
They came up ealier with gcc 12 (Windows), but apparently gcc 14 is
still reporting them, also under Linux.

```
/home/runner/work/curl-for-win/curl-for-win/curl/lib/cf-h1-proxy.c: In function 'cf_h1_proxy_close':
/home/runner/work/curl-for-win/curl-for-win/curl/lib/cf-h1-proxy.c:1060:17: warning: null pointer dereference [-Wnull-dereference]
 1060 |   cf->connected = FALSE;
/home/runner/work/curl-for-win/curl-for-win/curl/lib/cf-h1-proxy.c:1061:8: warning: null pointer dereference [-Wnull-dereference]
 1061 |   if(cf->ctx) {
      |      ~~^~~~~
In function 'tunnel_free',
    inlined from 'cf_h1_proxy_destroy' at /home/runner/work/curl-for-win/curl-for-win/curl/lib/cf-h1-proxy.c:1053:3:
/home/runner/work/curl-for-win/curl-for-win/curl/lib/cf-h1-proxy.c:198:27: warning: null pointer dereference [-Wnull-dereference]
  198 |   struct h1_tunnel_state *ts = cf->ctx;
      |                           ^~
```
Ref: https://github.com/curl/curl-for-win/actions/runs/8985369476/job/24679219528#step:3:6320

Fixes curl#13237
Close #xxxxx
vszakats added a commit that referenced this issue May 8, 2024
They came up ealier with gcc 12 (Windows), but apparently gcc 14 is
still reporting them, also under Linux.

```
/home/runner/work/curl-for-win/curl-for-win/curl/lib/cf-h1-proxy.c: In function 'cf_h1_proxy_close':
/home/runner/work/curl-for-win/curl-for-win/curl/lib/cf-h1-proxy.c:1060:17: warning: null pointer dereference [-Wnull-dereference]
 1060 |   cf->connected = FALSE;
/home/runner/work/curl-for-win/curl-for-win/curl/lib/cf-h1-proxy.c:1061:8: warning: null pointer dereference [-Wnull-dereference]
 1061 |   if(cf->ctx) {
      |      ~~^~~~~
In function 'tunnel_free',
    inlined from 'cf_h1_proxy_destroy' at /home/runner/work/curl-for-win/curl-for-win/curl/lib/cf-h1-proxy.c:1053:3:
/home/runner/work/curl-for-win/curl-for-win/curl/lib/cf-h1-proxy.c:198:27: warning: null pointer dereference [-Wnull-dereference]
  198 |   struct h1_tunnel_state *ts = cf->ctx;
      |                           ^~
```
Ref: https://github.com/curl/curl-for-win/actions/runs/8985369476/job/24679219528#step:3:6320

Fixes #13237
Closes #13555
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants