romenskiy2012/curl
1
0
Fork 0
mirror of https://github.com/curl/curl.git synced 2026-04-14 18:11:40 +03:00
Code Issues Projects Releases Packages Wiki Activity

clang-tidy: fixes and improvements

Browse source 
Fix bigger and smaller kinks in how clang-tidy is configured and used.
Sync behavior more between autotools and cmake, lib/src and tests. Bump
clang-tidy minimum version and prepare logic to allow using clang-tidy
to a fuller extent.

- move clang-tidy settings from builds to a new `.clang-tidy.yml`.
  To make it easy to see and edit checks at one place. Also to allow
  using the `--checks=` option internally to silence tests-specific
  checks. (clang-tidy does not support multiple `--check=` options via
  the command-line.)
  Use explicit `--config-file=` option to point to the configuration.
- .clang-tidy.yml: link to documentation.
- suppress `clang-diagnostic-nullability-extension` due to a false
  positive in libtests with `CURL_WERROR=ON` and `PICKY_COMPILER=OFF`.
- .clang-tidy.yml: enable `portability-*`, `misc-const-correctness`.
- drop `--quiet` clang-tidy option by default to make its working a bit
  more transparent. The extra output is minimial.
- consistently use double-dashes in clang-tidy command-line options.
  Supported by clang-tidy 9.0.0+ (2019-09-19). Before this patch single
  and double were used arbitrarily.
- src/tool_parsecfg: silence false positive `clang-analyzer-unix.Stream`.
  Seen with clang 18 + clang-tidy 19 and 20 (only with autotools.)
- INTERNALS: require clang-tidy 14.0.0+. For the `--config-file` option.
- INTERNALS: recommend clang-tidy 19.1.0+, to avoid bogus
  `clang-analyzer-valist.Uninitialized` warnings. (bug details below)

autotools:

- allow configuring the clang-tidy tool via `CLANG_TIDY` env.
  Also to use in GHA to point to a suffixed clang-tody tool.
- fix to pass CFLAGS to lib, src sources.
  (keep omitting them when using a non-clang compiler.)
- fix to pass `--warnings-as-errors=*` in quotes to avoid globbing.

cmake:

- fix to not pass an empty `-I` to clang-tidy.
- fix to pass CFLAGS (picky warnings) to clang-tidy for test sources.
  (keep omitting them when using a non-clang compiler.)
- fix to disable `clang-diagnostic-unused-function` for test sources.
  (tests have static entry points, which trigger this check when
  checking them as individidual sources.)
- fix forwarding `CURL_CLANG_TIDYFLAGS` to clang-tidy.
- force disable picky warnings when running clang-tidy with a non-clang
  compiler. To not pass these flags when checking lib and src.

CI:

- GHA/linux: avoid clang-tidy bug by upgrading to v19, and drop the
  workaround.
- GHA/linux: switch to clang from gcc in the clang-tidy job. Using gcc
  doesn't allow passing CFLAGS to clang-tidy, making it less effective.
  (My guess this was one factor contributing to this job often missing
  to find certain issues compared to GHA/macos.)

I recomment using clang-tidy with a clang compiler, preferably the same
version or one that's compatible. Other cases are best effort, and may
fail if a C flag is passed to clang-tidy that it does not understand.
Picky warnings are mostly omitted when using a non-clang compiler,
reducing its usefulness.

Details and reproducer for the v18 (and earlier) clang-tidy bug,
previously affecting the GHA/linux job:

clang-tidy <=18 emits false warnings way when passing multiple C sources
at once (as done with autotools):

```sh
cat > src1.c <<EOF
#include <string.h>
static void dummy(void *p) { memcmp(p, p, 0); }
EOF

cat > src2.c <<EOF
#include <stdarg.h>
void vafunc(int option, ...)
{
  va_list param;
  va_start(param, option);
  if(option)
    (void)va_arg(param, int);
  va_end(param);
}
EOF

/opt/homebrew/opt/llvm@18/bin/clang-tidy --checks=clang-analyzer-valist.Uninitialized src1.c src2.c

# src2.c:7:11: warning: va_arg() is called on an uninitialized va_list [clang-analyzer-valist.Uninitialized]
```

Follow-up to e86542038d #17047

Closes #20605
This commit is contained in:
Viktor Szakats 2026-02-15 23:58:39 +01:00
parent 5fa5cb3825
commit 4497dbd9ac
No known key found for this signature in database
10 changed files with 84 additions and 30 deletions
Download patch file Download diff file Expand all files Collapse all files

16
.clang-tidy.yml Normal file
View file

@ -0,0 +1,16 @@
# Copyright (C) Daniel Stenberg, <daniel@haxx.se>, et al.
#
# SPDX-License-Identifier: curl
---
# https://clang.llvm.org/extra/clang-tidy/checks/list.html
# -clang-analyzer-security.insecureAPI.bzero: for FD_ZERO() (seen on macOS)
# -clang-analyzer-security.ArrayBound: due to false positives with clang-tidy v21.1.0
Checks: >-
-clang-analyzer-security.insecureAPI.bzero,
-clang-analyzer-optin.performance.Padding,
-clang-analyzer-security.ArrayBound,
-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,
-clang-diagnostic-nullability-extension,
misc-const-correctness,
portability-*

5
.github/workflows/linux.yml vendored
View file

@ -33,7 +33,6 @@ env:
MAKEFLAGS: -j 5
CURL_CI: github
CURL_TEST_MIN: 1600
CURL_CLANG_TIDYFLAGS: '-checks=-clang-analyzer-security.insecureAPI.bzero,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.ArrayBound,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,-clang-analyzer-valist.Uninitialized'
# renovate: datasource=github-tags depName=libressl/portable versioning=semver registryUrl=https://github.com
LIBRESSL_VERSION: 4.2.1
# renovate: datasource=github-tags depName=wolfSSL/wolfssl versioning=semver extractVersion=^v?(?<version>.+)-stable$ registryUrl=https://github.com
@ -277,13 +276,15 @@ jobs:
-DCURL_DISABLE_LDAP=ON -DUSE_LIBIDN2=OFF -DCURL_USE_LIBSSH2=OFF
- name: 'clang-tidy'
install_packages: clang-tidy libssl-dev libidn2-dev libssh2-1-dev libnghttp2-dev libldap-dev libkrb5-dev librtmp-dev libgnutls28-dev
install_packages: clang-19 clang-tidy-19 libssl-dev libidn2-dev libssh2-1-dev libnghttp2-dev libldap-dev libkrb5-dev librtmp-dev libgnutls28-dev
install_steps: skipall mbedtls-latest-intel rustls wolfssl-opensslextra-intel
install_steps_brew: gsasl
make-custom-target: tidy
LDFLAGS: -Wl,-rpath,/home/runner/wolfssl-opensslextra/lib -Wl,-rpath,/home/linuxbrew/.linuxbrew/opt/gsasl/lib
PKG_CONFIG_PATH: /home/linuxbrew/.linuxbrew/opt/gsasl/lib/pkgconfig
CC: clang-19
configure: >-
CLANG_TIDY=clang-tidy-19
--with-wolfssl=/home/runner/wolfssl-opensslextra --with-openssl --with-rustls --with-mbedtls=/home/runner/mbedtls --with-gnutls --with-libgsasl
--with-librtmp --with-libssh2 --with-libidn2
--enable-ech --with-gssapi --enable-ssls-export --disable-typecheck

32
CMake/Macros.cmake
View file

@ -115,6 +115,10 @@ macro(curl_collect_target_options _target)
if(_val)
list(APPEND _definitions ${_val})
endif()
get_target_property(_val ${_target} COMPILE_OPTIONS)
if(_val)
list(APPEND _options ${_val})
endif()
get_target_property(_val ${_target} LINK_LIBRARIES)
if(_val)
foreach(_lib IN LISTS _val)
@ -132,6 +136,7 @@ macro(curl_add_clang_tidy_test_target _target_clang_tidy _target)
set(_includes "")
set(_definitions "")
set(_options "")
# Collect header directories and macro definitions applying to the directory
get_directory_property(_val INCLUDE_DIRECTORIES)
@ -142,13 +147,24 @@ macro(curl_add_clang_tidy_test_target _target_clang_tidy _target)
if(_val)
list(APPEND _definitions ${_val})
endif()
get_directory_property(_val COMPILE_OPTIONS)
if(_val)
list(APPEND _options ${_val})
endif()
unset(_val)
# Collect header directories and macro definitions from lib dependencies
curl_collect_target_options(${_target})
list(REMOVE_ITEM _includes "")
string(REPLACE ";" ";-I" _includes ";${_includes}")
set(_includes_tmp ${_includes})
set(_includes)
foreach(_inc IN LISTS _includes_tmp)
# Avoid empty and '$<INSTALL_INTERFACE:include>' items. The latter also
# evaluates to an empty path in this context.
if(_inc AND NOT _inc MATCHES "INSTALL_INTERFACE:")
list(APPEND _includes "-I${_inc}")
endif()
endforeach()
list(REMOVE_DUPLICATES _includes)
list(REMOVE_ITEM _definitions "")
@ -156,6 +172,12 @@ macro(curl_add_clang_tidy_test_target _target_clang_tidy _target)
list(REMOVE_DUPLICATES _definitions)
list(SORT _definitions) # Sort like CMake does
if(CMAKE_C_COMPILER_ID MATCHES "Clang")
list(REMOVE_DUPLICATES _options) # Keep the first of duplicates to imitate CMake
else()
set(_options)
endif()
# Assemble source list
set(_sources "")
foreach(_source IN ITEMS ${ARGN})
@ -165,14 +187,18 @@ macro(curl_add_clang_tidy_test_target _target_clang_tidy _target)
list(APPEND _sources "${_source}")
endforeach()
# Pass -clang-diagnostic-unused-function to disable -Wunused-function implied by -Wunused
add_custom_target(${_target_clang_tidy} USES_TERMINAL
WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
COMMAND ${CMAKE_C_CLANG_TIDY} ${_sources} -- ${_includes} ${_definitions}
COMMAND ${CMAKE_C_CLANG_TIDY}
"--checks=-clang-diagnostic-unused-function"
${_sources} -- ${_includes} ${_definitions} ${_options}
DEPENDS ${_sources})
add_dependencies(tests-clang-tidy ${_target_clang_tidy})
unset(_includes)
unset(_definitions)
unset(_options)
unset(_sources)
endif()
endmacro()

23
CMakeLists.txt
View file

@ -251,8 +251,6 @@ if(NOT DOS AND NOT AMIGA)
option(ENABLE_THREADED_RESOLVER "Enable threaded DNS lookup" ${_enable_threaded_resolver_default})
endif()
include(PickyWarnings)
if(CYGWIN OR CMAKE_SYSTEM_NAME STREQUAL "Linux" OR CMAKE_SYSTEM_NAME STREQUAL "GNU")
set_property(DIRECTORY APPEND PROPERTY COMPILE_DEFINITIONS "_GNU_SOURCE") # Required for accept4(), pipe2(), sendmmsg()
list(APPEND CMAKE_REQUIRED_DEFINITIONS "-D_GNU_SOURCE") # Apply to all feature checks
@ -270,22 +268,21 @@ endif()
option(CURL_CLANG_TIDY "Run the build through clang-tidy" OFF)
if(CURL_CLANG_TIDY)
find_program(CLANG_TIDY NAMES "clang-tidy" REQUIRED)
if(NOT CMAKE_C_COMPILER_ID MATCHES "Clang")
set(PICKY_COMPILER OFF) # Do a best effort and skip passing non-clang warning options to clang-tidy.
# This lets through warning options enabled via CURL_WERROR=ON, affecting lib and src.
endif()
set(CMAKE_UNITY_BUILD OFF) # clang-tidy is not looking into #included sources, thus not compatible with unity builds.
set(CURL_DISABLE_TYPECHECK ON) # to improve performance and avoid potential interference.
set(_tidy_checks "")
list(APPEND _tidy_checks "-clang-analyzer-security.insecureAPI.bzero") # for FD_ZERO() (seen on macOS)
list(APPEND _tidy_checks "-clang-analyzer-optin.performance.Padding")
list(APPEND _tidy_checks "-clang-analyzer-security.ArrayBound") # false positives with clang-tidy v21.1.0
list(APPEND _tidy_checks "-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling")
string(REPLACE ";" "," _tidy_checks "${_tidy_checks}")
find_program(CLANG_TIDY NAMES "clang-tidy" REQUIRED)
set(CMAKE_C_CLANG_TIDY "${CLANG_TIDY}" "-checks=${_tidy_checks}" "-quiet")
unset(_tidy_checks)
set(CMAKE_C_CLANG_TIDY "${CLANG_TIDY}")
list(APPEND CMAKE_C_CLANG_TIDY "--config-file=${PROJECT_SOURCE_DIR}/.clang-tidy.yml")
if(CURL_WERROR)
list(APPEND CMAKE_C_CLANG_TIDY "--warnings-as-errors=*")
endif()
if(CURL_CLANG_TIDYFLAGS)
list(APPEND CMAKE_C_CLANG_TIDY ${CURL_CLANG_TIDYFLAGS})
string(REPLACE " " ";" _tidy_flags_list "${CURL_CLANG_TIDYFLAGS}")
list(APPEND CMAKE_C_CLANG_TIDY ${_tidy_flags_list})
endif()
endif()
@ -309,6 +306,8 @@ if(CURL_CODE_COVERAGE)
endif()
endif()
include(PickyWarnings)
set(CURL_CFLAGS "") # C flags set for libcurl and curl tool (aka public binaries) only
option(CURL_DROP_UNUSED "Drop unused code and data from built binaries" OFF)

2
Makefile.am
View file

@ -64,7 +64,7 @@ CMAKE_DIST = \
tests/cmake/test.c \
tests/cmake/test.sh
EXTRA_DIST = CHANGES.md COPYING RELEASE-NOTES Dockerfile .editorconfig $(CMAKE_DIST)
EXTRA_DIST = CHANGES.md COPYING RELEASE-NOTES Dockerfile .clang-tidy.yml .editorconfig $(CMAKE_DIST)
DISTCLEANFILES = buildinfo.txt

7
configure.ac
View file

@ -120,6 +120,11 @@ AC_SUBST([AR])
AC_SUBST(libext)
if test -z "$CLANG_TIDY"; then
CLANG_TIDY=clang-tidy
fi
AC_SUBST(CLANG_TIDY)
dnl figure out the libcurl version
CURLVERSION=`$SED -ne 's/^#define LIBCURL_VERSION "\(.*\)".*/\1/p' ${srcdir}/include/curl/curlver.h`
XC_CHECK_PROG_CC
@ -681,6 +686,8 @@ case $host_os in
;;
esac
AM_CONDITIONAL(CLANG, test "$compiler_id" = "APPLECLANG" || test "$compiler_id" = "CLANG")
dnl **********************************************************************
dnl Compilation based checks should not be done before this point.
dnl **********************************************************************

1
docs/INTERNALS.md
View file

@ -49,6 +49,7 @@ When writing code (mostly for generating stuff included in release tarballs)
we use a few "build tools" and we make sure that we remain functional with
these versions:
- clang-tidy 14.0.0 (2022-03-23), recommended: 19.1.0 or later (2024-09-17)
- cmake 3.7 (2016-11-11)
- GNU autoconf 2.59 (2003-11-06)
- GNU automake 1.7 (2002-09-25)

13
lib/Makefile.am
View file

@ -163,17 +163,18 @@ UNITV_ = $(UNITV_0)
$(UNITPROTOS): $(CSOURCES)
$(UNIT_V)(cd $(srcdir) && @PERL@ ../scripts/extract-unit-protos $(CSOURCES)) > $(top_builddir)/lib/$(UNITPROTOS)
# disable the tests that are mostly causing false positives
TIDYFLAGS := -checks=-clang-analyzer-security.insecureAPI.bzero,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.ArrayBound,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling -quiet
_tidy_cflags :=
TIDYFLAGS :=
if CURL_WERROR
TIDYFLAGS += --warnings-as-errors=*
TIDYFLAGS += '--warnings-as-errors=*'
endif
if CLANG
_tidy_cflags += $(CFLAGS)
endif
TIDY := clang-tidy
tidy:
(_csources=`echo ' $(CSOURCES)' | sed -E -e 's/ +$$//' -e 's/ +/ /g' -e 's| | $(srcdir)/|g'`; \
$(TIDY) $$_csources $(TIDYFLAGS) $(CURL_CLANG_TIDYFLAGS) -- $(AM_CPPFLAGS) $(CPPFLAGS) -DHAVE_CONFIG_H)
@CLANG_TIDY@ --config-file=$(top_srcdir)/.clang-tidy.yml $(TIDYFLAGS) $(CURL_CLANG_TIDYFLAGS) $$_csources -- $(AM_CPPFLAGS) $(CPPFLAGS) -DHAVE_CONFIG_H $(_tidy_cflags))
optiontable:
@PERL@ $(srcdir)/optiontable.pl < $(top_srcdir)/include/curl/curl.h > $(srcdir)/easyoptions.c

13
src/Makefile.am
View file

@ -215,17 +215,18 @@ all-local: checksrc
endif
endif
# disable the tests that are mostly causing false positives
TIDYFLAGS := -checks=-clang-analyzer-security.insecureAPI.bzero,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.ArrayBound,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling -quiet
_tidy_cflags :=
TIDYFLAGS =
if CURL_WERROR
TIDYFLAGS += --warnings-as-errors=*
TIDYFLAGS += '--warnings-as-errors=*'
endif
if CLANG
_tidy_cflags += $(CFLAGS)
endif
TIDY := clang-tidy
tidy: $(HUGE) $(CA_EMBED_CSOURCE)
(_curl_cfiles=`echo ' $(CURL_CFILES)' | sed -e 's/ +/ /g' -e 's| | $(srcdir)/|g'`; \
$(TIDY) $$_curl_cfiles $(curl_cfiles_gen) $(TIDYFLAGS) $(CURL_CLANG_TIDYFLAGS) -- $(curl_CPPFLAGS) $(CPPFLAGS) $(AM_CPPFLAGS) -DHAVE_CONFIG_H)
@CLANG_TIDY@ --config-file=$(top_srcdir)/.clang-tidy.yml $(TIDYFLAGS) $(CURL_CLANG_TIDYFLAGS) $$_curl_cfiles $(curl_cfiles_gen) -- $(curl_CPPFLAGS) $(CPPFLAGS) $(AM_CPPFLAGS) -DHAVE_CONFIG_H $(_tidy_cflags))
listhelp:
(cd $(top_srcdir)/docs/cmdline-opts && make listhelp)

2
src/tool_parsecfg.c
View file

@ -253,6 +253,8 @@ ParameterError parseconfig(const char *filename, int max_recursive,
curlx_dyn_free(&pbuf);
if(file != stdin)
curlx_fclose(file);
/* Silence false positive about failing to close stdin.
NOLINTNEXTLINE(clang-analyzer-unix.Stream) */
if(fileerror)
err = PARAM_READ_ERROR;
}