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

use new gcc warnings #2463

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
@bagder
Member

bagder commented Apr 6, 2018

Continued work on @cooljeanius' take from #2307 with my added polish to make sure they build fine all over.

cooljeanius and others added some commits Feb 12, 2018

curl_setup: provide a CURL_SA_FAMILY_T type if none exists
... and use this type instead of 'sa_family_t' in the code since several
platforms don't have it.

@bagder bagder added the build label Apr 6, 2018

@cooljeanius

This comment has been minimized.

Show comment
Hide comment
@cooljeanius

cooljeanius Apr 6, 2018

Contributor

Thank you!

Contributor

cooljeanius commented Apr 6, 2018

Thank you!

@bagder bagder closed this in 8020a0c Apr 7, 2018

@bagder bagder deleted the bagder/new-gcc-warnings branch Apr 7, 2018

@MarcelRaad

This comment has been minimized.

Show comment
Hide comment
@MarcelRaad

MarcelRaad Apr 8, 2018

Member

Now I get:

http.c: In function 'Curl_copy_header_value':
http.c:220:9: error: potential null pointer dereference [-Werror=null-dereference]
   while(*header && (*header != ':'))

We aren't annotating parameters as nonnull, are we?

Member

MarcelRaad commented Apr 8, 2018

Now I get:

http.c: In function 'Curl_copy_header_value':
http.c:220:9: error: potential null pointer dereference [-Werror=null-dereference]
   while(*header && (*header != ':'))

We aren't annotating parameters as nonnull, are we?

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Apr 8, 2018

Member

We aren't annotating parameters as nonnull, are we?

No... it rather makes me suspect it uses the assert() above as a hint, but I'm a bit puzzled. This function really is not possible to call with a NULL pointer in the current code, I checked all the current call paths.

Member

bagder commented Apr 8, 2018

We aren't annotating parameters as nonnull, are we?

No... it rather makes me suspect it uses the assert() above as a hint, but I'm a bit puzzled. This function really is not possible to call with a NULL pointer in the current code, I checked all the current call paths.

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Apr 8, 2018

Member

btw, which compiler (version) gives you that warning?

Member

bagder commented Apr 8, 2018

btw, which compiler (version) gives you that warning?

@MarcelRaad

This comment has been minimized.

Show comment
Hide comment
@MarcelRaad

MarcelRaad Apr 8, 2018

Member

This is MinGW-w64's GCC 7.3:

Target: i686-w64-mingw32
Configured with: ../gcc-7.3.0/configure --prefix=/mingw32 --with-local-prefix=/mingw32/local --build=i686-w64-mingw32 --host=i686-w64-mingw32 --target=i686-w64-mingw32 --with-native-system-header-dir=/mingw32/i686-w64-mingw32/include --libexecdir=/mingw32/lib --enable-bootstrap --with-arch=i686 --with-tune=generic --enable-languages=c,lto,c++,objc,obj-c++,fortran,ada --enable-shared --enable-static --enable-libatomic --enable-threads=posix --enable-graphite --enable-fully-dynamic-string --enable-libstdcxx-time=yes --enable-libstdcxx-filesystem-ts=yes --disable-libstdcxx-pch --disable-libstdcxx-debug --disable-isl-version-check --enable-lto --enable-libgomp --disable-multilib --enable-checking=release --disable-rpath --disable-win32-registry --disable-nls --disable-werror --disable-symvers --with-libiconv --with-system-zlib --with-gmp=/mingw32 --with-mpfr=/mingw32 --with-mpc=/mingw32 --with-isl=/mingw32 --with-pkgversion='Rev1, Built by MSYS2 project' --with-bugurl=https://sourceforge.net/projects/msys2 --with-gnu-as --with-gnu-ld --disable-sjlj-exceptions --with-dwarf2
Thread model: posix
gcc version 7.3.0 (Rev1, Built by MSYS2 project)
Member

MarcelRaad commented Apr 8, 2018

This is MinGW-w64's GCC 7.3:

Target: i686-w64-mingw32
Configured with: ../gcc-7.3.0/configure --prefix=/mingw32 --with-local-prefix=/mingw32/local --build=i686-w64-mingw32 --host=i686-w64-mingw32 --target=i686-w64-mingw32 --with-native-system-header-dir=/mingw32/i686-w64-mingw32/include --libexecdir=/mingw32/lib --enable-bootstrap --with-arch=i686 --with-tune=generic --enable-languages=c,lto,c++,objc,obj-c++,fortran,ada --enable-shared --enable-static --enable-libatomic --enable-threads=posix --enable-graphite --enable-fully-dynamic-string --enable-libstdcxx-time=yes --enable-libstdcxx-filesystem-ts=yes --disable-libstdcxx-pch --disable-libstdcxx-debug --disable-isl-version-check --enable-lto --enable-libgomp --disable-multilib --enable-checking=release --disable-rpath --disable-win32-registry --disable-nls --disable-werror --disable-symvers --with-libiconv --with-system-zlib --with-gmp=/mingw32 --with-mpfr=/mingw32 --with-mpc=/mingw32 --with-isl=/mingw32 --with-pkgversion='Rev1, Built by MSYS2 project' --with-bugurl=https://sourceforge.net/projects/msys2 --with-gnu-as --with-gnu-ld --disable-sjlj-exceptions --with-dwarf2
Thread model: posix
gcc version 7.3.0 (Rev1, Built by MSYS2 project)
@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Apr 8, 2018

Member

Interestingly, I don't get that error with gcc 7.3.0 on Linux (7.3.0-15 on debian).

Member

bagder commented Apr 8, 2018

Interestingly, I don't get that error with gcc 7.3.0 on Linux (7.3.0-15 on debian).

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Apr 8, 2018

Member

@MarcelRaad: does the warning go away if you remove the assert? Or do you need to add a "real" check for NULL there to make it go away?

Member

bagder commented Apr 8, 2018

@MarcelRaad: does the warning go away if you remove the assert? Or do you need to add a "real" check for NULL there to make it go away?

@MarcelRaad

This comment has been minimized.

Show comment
Hide comment
@MarcelRaad

MarcelRaad Apr 9, 2018

Member

Yes, removing the assert makes the warning go away! And there's also one in curlx_strtoofft that goes away after removing the assert.

Member

MarcelRaad commented Apr 9, 2018

Yes, removing the assert makes the warning go away! And there's also one in curlx_strtoofft that goes away after removing the assert.

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Apr 9, 2018

Member

Ah, will you land removals of those asserts then? I don't think those are terribly valuable anyway since if we do get NULL pointers there we will crash/trigger valgrind reports etc.

Member

bagder commented Apr 9, 2018

Ah, will you land removals of those asserts then? I don't think those are terribly valuable anyway since if we do get NULL pointers there we will crash/trigger valgrind reports etc.

@MarcelRaad

This comment has been minimized.

Show comment
Hide comment
@MarcelRaad

MarcelRaad Apr 9, 2018

Member

OK, will do!

Member

MarcelRaad commented Apr 9, 2018

OK, will do!

MarcelRaad added a commit that referenced this pull request Apr 9, 2018

lib: silence null-dereference warnings
In debug mode, MingGW-w64's GCC 7.3 issues null-dereference warnings
when dereferencing pointers after DEBUGASSERT-ing that they are not
NULL.
Fix this by removing the DEBUGASSERTs.

Suggested-by: Daniel Stenberg
Ref: #2463

@lock lock bot locked as resolved and limited conversation to collaborators Jul 8, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.