This also means that we no longer pass any picky warning option to
ossfuzz. It's probably not worth maintaining picky C++ options for this
single, small target.
Silencing:
```
cc1plus: warning: command-line option '-Wbad-function-cast' is valid for C/ObjC but not for C++
cc1plus: warning: command-line option '-Wdeclaration-after-statement' is valid for C/ObjC but not for C++
cc1plus: warning: command-line option '-Wenum-int-mismatch' is valid for C/ObjC but not for C++
cc1plus: warning: command-line option '-Wjump-misses-init' is valid for C/ObjC but not for C++
cc1plus: warning: command-line option '-Wmissing-parameter-type' is valid for C/ObjC but not for C++
cc1plus: warning: command-line option '-Wmissing-prototypes' is valid for C/ObjC but not for C++
cc1plus: warning: command-line option '-Wnested-externs' is valid for C/ObjC but not for C++
cc1plus: warning: command-line option '-Wold-style-declaration' is valid for C/ObjC but not for C++
cc1plus: warning: command-line option '-Wold-style-definition' is valid for C/ObjC but not for C++
cc1plus: warning: command-line option '-Wstrict-prototypes' is valid for C/ObjC but not for C++
```
Ref: https://github.com/libssh2/libssh2/actions/runs/18063134305/job/51402236388#step:7:15Closes#1686
- cmake: sync `-ftree-vrp` behavior with autotools.
- build: enable `-Wjump-misses-init` for GCC 4.5+.
Credits-to: Marcel Raad
- packet: fix `-Wjump-misses-init` warnings.
```
src/packet.c: In function ‘_libssh2_packet_add’:
src/packet.c:671:9: error: jump skips variable initialization [-Werror=jump-misses-init]
src/packet.c:920:31: note: ‘want_reply’ declared here
src/packet.c:671:9: error: jump skips variable initialization [-Werror=jump-misses-init]
src/packet.c:919:26: note: ‘len’ declared here
src/packet.c:669:9: error: jump skips variable initialization [-Werror=jump-misses-init]
src/packet.c:1121:31: note: ‘want_reply’ declared here
src/packet.c:669:9: error: jump skips variable initialization [-Werror=jump-misses-init]
src/packet.c:1120:26: note: ‘len’ declared here
src/packet.c:669:9: error: jump skips variable initialization [-Werror=jump-misses-init]
src/packet.c:1119:26: note: ‘channel’ declared here
```
- build: enable gcc-12/13+, clang-10+ picky warnings
- acinclude.m4: sync formatting/comments with curl.
- autotools: fix `-Wtrampolines` picky warning for gcc 4.x versions.
Follow-up to 854cfa8292#1524
- cmake: enable `-Wall` for MSVC when `PICKY_COMPILER=ON`.
- MSVC: fix `-Wall` warnings.
Seen on VS2015. Not seen on VS2022. Unknown for other versions.
```
tests\test_simple.c(60): warning C4777: 'fprintf' : format string '%d' requires an argument of type 'int', but variadic argument 1 has type 'std::size_t'
tests\test_simple.c(60): warning C4777: 'fprintf' : format string '%.*s' requires an argument of type 'int', but variadic argument 2 has type 'std::size_t'
```
- mbedtls: stop silencing warnings in 3rd-party header.
Follow-up to a3aa6b4ca8#1525
- cmake: stop deleting `-W<n>` from `CMAKE_C_FLAGS` (MSVC)
1. `CMAKE_C_FLAGS` may apply to other projects, and deleting/altering it
may be unexpected.
2. We pass `-W4`/`-Wall` internally now, which do override custom
`-W<n>` options as tested with VS2008 and newer VS generators.
Closes#1588
Also to sync up with similar `./configure` feature via
`CURL_ADD_COMPILER_WARNINGS()`.
Example: `-DCMAKE_C_FLAGS=-Wno-xor-used-as-pow`
It may be useful as a workaround if a specific build combination hits
a picky warning within libssh2's source code. If such happens, we do
appreciate a report to fix it in libssh2 itself.
Closes#1590
- fix `add_subdirectory` builds for old CMake versions.
- libssh2-config.cmake: fix to set CMP0099 for CMake 3.17+ only.
- libssh2-config.cmake: generalize code to support any number of deps.
(mainly to sync with curl.)
- libssh2-config.cmake: bind dependencies to the static libssh2 only.
Follow-up to a0d8529b08#1571
Follow-up to df0563a857#1535Closes#1581
Fixes:
```
CMake Error at bld-libssh2/_pkg/lib/cmake/libssh2/libssh2-config.cmake:35 (add_library):
add_library cannot create ALIAS target "libssh2::libssh2" because target
"libssh2::libssh2_shared" is imported but not globally visible.
Call Stack (most recent call first):
CMakeLists.txt:27 (find_package)
CMake Error at bld-libssh2/_pkg/lib/cmake/libssh2/libssh2-config.cmake:40 (add_library):
add_library cannot create ALIAS target "Libssh2::libssh2" because target
"libssh2::libssh2_shared" is imported but not globally visible.
Call Stack (most recent call first):
CMakeLists.txt:27 (find_package)
```
tests/cmake reproducer (requires #1581):
```shell
export CMAKE_CONSUMER=/path/to/CMake-3.12.0/bin/cmake
./test.sh find_package
```
I don't understand what this error says, why it happens in certain CMake
versions, and why a workaround is necessary for what seems like
a standard export/consume configuration. This patch is based on internet
suggestions and other projects ending up with this workaround.
Ref: https://github.com/curl/curl/pull/16973
Ref: https://github.com/curl/curl/pull/17140
Cherry-picked from #1581Closes#1587
Tidy up the way we handle options not to be passed to feature checks,
and make sure to show them in the configure log.
Also:
- drop redundant and obsolete compiler check.
- drop redundant parenthesis.
Closes#1576
- replace `CMAKE_C_FLAGS*` and `CMAKE_CXX_FLAGS` with `COMPILE_OPTIONS`.
- replace `CMAKE_SHARED_LINKER_FLAGS_DEBUG` with
`LINK_OPTIONS`/`LINK_FLAGS`.
- make it explicit to pass these C flags to feature checks.
- enable `-pedantic-errors` picky option for GCC with CMake <3.23.
- drop redundant condition when stripping existing MSVC `/Wn` options.
CMake passes `CMAKE_C_FLAGS` to targets, feature checks and raw
`try_compile()` calls. With `COMPILE_OPTIONS`, this is limited to
targets, and we must explicitly pass them to feature checks. This
makes the build logic clearer, and offers more control. It also
reduces log noise by omitting these options from linker commands,
and from `CMAKE_C_FLAGS` dumps in feature checks.
Closes#1575
- drop `VERSION` target property for cmake <3.19 compatibility
```
CMake Error at CMake/Find*.cmake:90 (set_target_properties):
INTERFACE_LIBRARY targets may only have whitelisted properties.
The property "VERSION" is not allowed.
```
- move custom target property to the `INTERFACE_` namespace
for cmake <3.19 compatibility. (To avoid same error as above)
- fix forwarding multiple `CFLAGS`, when detected via `pkg-config`.
- restore support for `-framework` and raw libs when processing
the internal lib list for generating `libssh2.pc`. For good measure,
at the moment libssh2 doesn't depend on a Framework.
- limit `libssh2_dumptargetprops()` to cmake 3.19+. It doesn't work with
older versions.
Issues found while applying this change to curl. They did not surface in
libssh2 CI.
Follow-up to df0563a857#1535Closes#1571
- move dependency properties (libs, libdirs, C flags, header dirs,
pkg-config module names) from global lists to imported target
`INTERFACE` properties. Rework FInd modules to return their results
like this and update the libssh2 build process to use it. It makes
Find modules re-usable from the cmake-config script by libssh2
consumers, to integrate with libssh2 dependencies.
- define libssh2 dependencies as "imported targets" by the name:
`libssh2::<depname>`, e.g. `libssh2::libgcrypt`.
- cmake-config: add fall-back logic for CMake without
CMP0099 (v3.17 2020-03-20) to set lib directories.
- generate `libssh2.pc` based on imported target properties (instead of
global lists).
- add target property dump debug function.
- ci/GHA: also test cmake integration on macOS.
Follow-up to 96d7f404e7#1534Closes#1535
CMake:
- Find*: set `<modulename>_FOUND` for compatibility when found via
`pkg-config`. E.g. `MbedTLS_FOUND`.
`find_package_handle_standard_args()` sets both `<MODULENAME>_FOUND`
and `<Modulename>_FOUND` when detecting the dependency. Some CMake
code relies on this and 3rd-party code may rely on it too. Make sure
to set the latter variant when detecting the dependency via
`pkg-config`, where we don't call
`find_package_handle_standard_args()`.
CMake sets these variable to `TRUE` (not `ON` or `1`). Replicate this
for compatibility.
- libssh2-config.cmake: inherit default `LIBSSH2_USE_PKGCONFIG`.
Follow-up to a3aa6b4ca8#1525
- document variables consumed by `libssh2-config.cmake.in`.
- `libssh2-config.cmake`: fix to link to non-OpenSSL crypto backends.
This is most likely not how this is supposed to be done, but better
than failing.
What's the canonical way to do this, and how OpenSSL and zlib does it
is yet to be figured out.
- use `ZLIB::ZLIB` to reference zlib.
- use `IN ITEMS` where missed.
- harmonize variable dump output formats.
CMake `find_package` integration tests:
- extend to all crypto backends (was: OpenSSL).
- show libssh2 variables set by `find_package()`.
- stop building examples and tests for the consumed package.
For performance.
- enable zlib, for coverage.
- be verbose when building the test targets.
ci/GHA:
- add packaged mbedTLS (2.x) build to Linux matrix.
- alphasort some tests.
Follow-up to d9c2e550ca#1460
Follow-up to 82b09f9b3a#1322Closes#1534
- enable compiler warning `-Wcast-qual`.
- add `LIBSSH2_UNCONST()` macro to strip const where absolutely
necessary to avoid compiler warnings.
- fix const stripping by constifying where necessary.
- fix const stripping by using `LIBSSH2_UNCONST()`.
- libgcrypt.h: drop unnecessary casts.
- openssl: fix to use new `BIO_new_mem_buf()` parameter types
with wolfSSL.
Cherry-picked from #1484Closes#1527
- show platform flags (via curl).
- add `LIBSSH2_USE_PKGCONFIG` option to control whether to use
`pkg-config` to find dependencies.
- set `.pc` names withing the Find modules.
- add `mbedcrypto` to `libssh2.pc` only when detected via `pkg-config`.
Workaround for older mbedtls versions and non-CMake mbedTLS builds
(as of mbedTLS 3.6.2) that don't emit an `mbedcrypto.pc` file.
- set header paths relative to the project root (tidy-up).
- use `-isystem` for crypto backend and zlib header paths.
To match autotools.
- sync header path order with autotools.
- rename local variables to underscore-lowercase.
- minor tidy-ups.
Cherry-picked from #1484Closes#1525
- cmake: add support to build ossfuzz.
Enable with `-DBUILD_OSSFUZZ=ON`.
Also supports `-DLIB_FUZZING_ENGINE=` like autotools does.
- check for `__clang__` when suppressing warnings in source. Necessary
for clang-cl, which set `__clang__`, but doesn't set `__GNU__`.
- cmake: optimize out 4 picky warning option detections with gcc.
- cmake: bring `-pedantic-error`, `-Wall` use closer to curl's.
- cmake: set `-Wno-language-extension-token` for clang-cl.
- cmake: escape only the necessary `-W` options for clang-cl.
- cmake: apply picky warnings to C++.
- cmake: replace `unset(VAR)` with `set(VAR "")` for init.
- cmake: prefer dash-style MSVC options.
- cmake: simplify `MATCHES` expression.
- cmake: formatting/whitespace.
- ci/GHA: bump `actions/upload-artifact` to v4
Closes#1524
```
CMake Warning (dev) at cmake/CopyRuntimeDependencies.cmake:59 (add_custom_command):
The following keywords are not supported when using
add_custom_command(TARGET): DEPENDS.
Policy CMP0175 is not set: add_custom_command() rejects invalid arguments.
Run "cmake --help-policy CMP0175" for policy details. Use the cmake_policy
command to set the policy and suppress this warning.
Call Stack (most recent call first):
example/CMakeLists.txt:58 (add_target_to_copy_dependencies)
```
```
CMake Warning (dev) at cmake/CopyRuntimeDependencies.cmake:59 (add_custom_command):
Exactly one of PRE_BUILD, PRE_LINK, or POST_BUILD must be given. Assuming
POST_BUILD to preserve backward compatibility.
Policy CMP0175 is not set: add_custom_command() rejects invalid arguments.
Run "cmake --help-policy CMP0175" for policy details. Use the cmake_policy
command to set the policy and suppress this warning.
Call Stack (most recent call first):
example/CMakeLists.txt:58 (add_target_to_copy_dependencies)
```
Ref: https://github.com/libssh2/libssh2/actions/runs/12614228505/job/35152908184?pr=1510#step:15:77
This warning remains silent in unity builds. Since we're using unity
in CI for most jobs, warnings remain undetected there.
Disable them for all builds to avoid a surprise warning outside our CI.
It also doesn't work in any #included `.c` sources, like the crypto
backend sources.
The issue caught by the warning is useful for a tidy codebase, but
doesn't affect executed code. It was enabled in
84338c4de2d7c798e3c270c9610d51a4ad18a90b #12331 (2023-11-15).
llvm source: fee2953f23/clang/lib/Sema/AnalysisBasedWarnings.cpp (L125-L134)
llvm issue: https://github.com/llvm/llvm-project/issues/71046
Follow-up to 7ecc309cd1#1224Closes#1481
`pkg_check_modules()` seems to leave `<PACKAGE>_VERSION` defined with an
empty value, if the package is not found.
When the package is also not found in the fallback branch,
`find_package_handle_standard_args()` logs and error message. In this
message it includes the bogus empty value as: `(found version "")`:
```
Could NOT find Libgcrypt (missing: LIBGCRYPT_INCLUDE_DIR LIBGCRYPT_LIBRARY)
(found version "")
```
Clear the version number to avoid the confusion:
```
Could NOT find Libgcrypt (missing: LIBGCRYPT_INCLUDE_DIR LIBGCRYPT_LIBRARY)
```
Seen with CMake v3.30.3 and 3.30.5.
Closes#1479
- sync code between Find modules.
- wolfssl: replace `pkg-config` hints with native detection.
- libgcrypt, mbedtls: add `pkg-config`-based native detection.
- libgcrypt: add version detection.
- limit `pkg-config` use for `UNIX`, vcpkg, and non-cross MinGW builds,
and builds with no manual customization via `*_INCLUDE_DIR` or
`*_LIBRARY`.
- replace and sync Find module header comments.
- ci: delete manual mbedTLS config that's now redundant.
Based on similar work done in curl.
Second attempt at #1420Closes#1445
* Use the imported target of FindOpenSSL module
* Build libssh2 before test runner
* Use find_package() in the CMake config file
* Use find_dependency() rather than find_package()
* Install CMake module files and use them in the config file
* Use elseif() to choose the crypto backend
Ref:
https://best.openssf.org/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.html
(2023-11-29)
Enable new warnings:
- replace `-Wno-sign-conversion` with `-Wsign-conversion`.
Fix them in example, tests and wincng. There remain about 360 of these
warnings in `src`. Add a TODO item for those and disable `-Werror` for
this particular warning.
- enable `-Wformat=2` for clang (in both cmake and autotools).
- enable `__attribute__((format))` for `_libssh2_debug()`,
`_libssh2_snprintf()` and in tests for `run_command()`.
`LIBSSH2_PRINTF()` copied from `CURL_TEMP_PRINTF()` in curl.
- enable `-Wimplicit-fallthrough`.
- enable `-Wtrampolines`.
Fix them:
- src: replace obsolete fall-through-comments with
`__attribute__((fallthrough))`.
- wincng: fix `-Wsign-conversion` warnings.
- tests: fix `-Wsign-conversion` warnings.
- example: fix `-Wsign-conversion` warnings.
- src: fix `-Wformat` issues in trace calls.
Also, where necessary fix `int` and `unsigned char` casts to
`unsigned int` and adjust printf format strings. These were not
causing compiler warnings.
Cast large types to `long` to avoid dealing with printf masks for
`size_t` and other C99 types. Existing code often used `int` for this.
I'll update them to `long` in an upcoming commit.
- tests: fix `-Wformat` warning.
- silence `-Wformat-nonliteral` warnings.
- mbedtls: silence `-Wsign-conversion`/`-Warith-conversion`
in external header.
Closes#1257
Then sync this warning option with curl.
Seems like a false positive and/or couldn't figure how to fix it, so silence:
```
example/ssh2.c:227:38: error: '%s' directive output may be truncated writing likely 1 or more bytes into a region of size 0 [-Werror=format-truncation=]
227 | snprintf(fn1, fn1sz, "%s/%s", h, pubkey);
| ^~
example/ssh2.c:227:34: note: assuming directive output of 1 byte
227 | snprintf(fn1, fn1sz, "%s/%s", h, pubkey);
| ^~~~~~~
example/ssh2.c:227:13: note: 'snprintf' output 3 or more bytes (assuming 4) into a destination of size 2
227 | snprintf(fn1, fn1sz, "%s/%s", h, pubkey);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
example/ssh2.c:228:38: error: '%s' directive output may be truncated writing likely 1 or more bytes into a region of size 0 [-Werror=format-truncation=]
228 | snprintf(fn2, fn2sz, "%s/%s", h, privkey);
| ^~
example/ssh2.c:228:34: note: assuming directive output of 1 byte
228 | snprintf(fn2, fn2sz, "%s/%s", h, privkey);
| ^~~~~~~
example/ssh2.c:228:13: note: 'snprintf' output 3 or more bytes (assuming 4) into a destination of size 2
228 | snprintf(fn2, fn2sz, "%s/%s", h, privkey);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
Ref: https://github.com/libssh2/libssh2/actions/runs/7055480458/job/19205970397#step:10:98
Fix:
```
tests/openssh_fixture.c:116:38: error: ' 2>&1' directive output may be truncated writing 5 bytes into a region of size between 1 and 1024 [-Werror=format-truncation=]
tests/openssh_fixture.c:116:11: note: 'snprintf' output between 6 and 1029 bytes into a destination of size 1024
```
Ref: https://github.com/libssh2/libssh2/actions/runs/7055480458/job/19205969221#step:10:51
Tested via #1257
Enable more picky compiler warnings. I've found these options in the
nghttp3 project when implementing the CMake quick picky warning
functionality for it.
Fix issues found along the way:
- wincng, mbedtls: delete duplicate function declarations.
Most of this was due to re-#defining crypto functions to
crypto-backend specific implementations These redefines also remapped
the declarations in `crypto.h`, making the backend-specific
declarations duplicates.
This patch deletes the backend-specific declarations.
- wincng mapped two crypto functions to the same local function.
Also causing double declarations.
Fix this by adding two disctinct wrappers and moving
the common function to a static one.
- delete unreachable `break;` statements.
- kex: disable macros when unused.
- agent: disable unused constants.
- mbedtls: disable double declaration warnings because public mbedTLS
headers trigger it. (with function `psa_set_key_domain_parameters`)
- crypto.h: formatting.
Ref: a70edb08e9/cmake/PickyWarningsC.cmakeCloses#1224
- cmake, autotools: sync picky gcc warnings with curl.
- cmake, autotools: add `-Wold-style-definition` for clang too.
- cmake, autotools: add comment for `-Wformat-truncation=1`.
- cmake: more precise version info for old clang options.
Closes#1219
Also extend our integration test double inclusion. It will still not
catch this case, because that requires
`cmake_minimum_required(VERSION 3.18)` or higher.
Fixes:
```
CMake Error at .../lib/cmake/libssh2/libssh2-config.cmake:8 (add_library):
add_library cannot create ALIAS target "libssh2::libssh2" because another
target with the same name already exists.
Call Stack (most recent call first):
CMakeLists.txt:24 (find_package)
CMake Error at .../lib/cmake/libssh2/libssh2-config.cmake:13 (add_library):
add_library cannot create ALIAS target "Libssh2::libssh2" because another
target with the same name already exists.
Call Stack (most recent call first):
CMakeLists.txt:24 (find_package)
```
Test to reproduce:
```cmake
cmake_minimum_required(VERSION 3.18) # must be 3.18 or higher
project(test)
find_package(libssh2 CONFIG)
find_package(libssh2 CONFIG) # fails
add_executable(test main.c)
target_link_libraries(test libssh2::libssh2)
```
Ref: https://cmake.org/cmake/help/latest/release/3.18.html#other-changes
Ref: https://cmake.org/cmake/help/v3.18/policy/CMP0107.html
Assisted-by: Kai Pastor
Assisted-by: Harry Mallon
Ref: https://github.com/curl/curl/pull/11913Closes#1216
- lowercase, underscored local variables.
- fix `find_library()` to use the multiple names passed.
- rely more on `find_package_handle_standard_args()`.
Logic based on our `Findwolfssl.cmake`.
- delete ignored/unused `MBEDTLS_LIBRARY_DIR`.
- revert CI configuration to use `MBEDCRTYPO_LIBRARY`.
- clarify inputs/outputs in comment header.
- use variable for regex.
- formatting.
Follow-up to 4159467507#1192Closes#1196
- libssh2 needs the crypto lib only, stop dealing with the rest.
- simplify logic.
- drop hard-wired toolchain specific options that broke with e.g. MSVC.
Reported by: AR Visions
Fixes#1191
- add mbedTLS version detection for recent releases.
- merge custom detection results display into a single line.
- shorten mbedTLS configuration in macOS CI job.
Used the curl mbedTLS detection logic for ideas:
a8c773845f/CMake/FindMbedTLS.cmakeCloses#1192