cmake: replace the way clang-tidy verifies tests, fix issues found

Replace existing `mk-unity.pl` `--embed` workaround with running `clang-tidy` manually on individual test source instead. This aligns with how clang-tidy works and removes `mk-unity.pl` from the solution. Also: - mqttd: fix potentially uninitialized buffer by zero filling it. ``` tests/server/mqttd.c:484:41: error: The left operand of '<<' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult,-warnings-as-errors] 484 | payload_len = (size_t)(buffer[10] << 8) | buffer[11]; | ^ [...] tests/server/mqttd.c:606:45: error: The left operand of '<<' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult,-warnings-as-errors] 606 | topiclen = (size_t)(buffer[1 + bytes] << 8) | buffer[2 + bytes]; | ^ ``` - sockfilt: fix potential out-of-bound pointer: ``` tests/server/sockfilt.c:1128:33: error: The 2nd argument to 'send' is a buffer with size 17010 but should be a buffer with size equal to or greater than the value of the 3rd argument (which is 18446744073709551615) [clang-analyzer-unix.StdCLibraryFunctions,-warnings-as-errors] 1128 | ssize_t bytes_written = swrite(sockfd, buffer, buffer_len); | ^ ``` - clang-tidy: suppress bogus `bzero()` warnings that happens inside the notorious `FD_ZERO()` macros, on macOS. Ref: https://github.com/curl/curl/pull/17680#issuecomment-2991730158 Closes #17705
2026-04-11 12:01:42 +08:00 · 2025-06-22 03:17:33 +02:00 · 2025-06-22 03:17:33 +02:00 · e088e10454
commit e088e10454
parent 9837dd429a
14 changed files with 111 additions and 50 deletions
--- a/.github/workflows/linux.yml
+++ b/.github/workflows/linux.yml
@ -38,7 +38,7 @@ permissions: {}
 env:
  MAKEFLAGS: -j 5
  CURL_CI: github
-  CURL_CLANG_TIDYFLAGS: '-checks=-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,-clang-analyzer-valist.Uninitialized'
+  CURL_CLANG_TIDYFLAGS: '-checks=-clang-analyzer-security.insecureAPI.bzero,-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,-clang-analyzer-valist.Uninitialized'
  # renovate: datasource=github-tags depName=libressl-portable/portable versioning=semver registryUrl=https://github.com
  LIBRESSL_VERSION: 4.1.0
  # renovate: datasource=github-tags depName=wolfSSL/wolfssl versioning=semver extractVersion=^v?(?<version>.+)-stable$ registryUrl=https://github.com
--- a/CMake/Macros.cmake
+++ b/CMake/Macros.cmake
@ -95,3 +95,71 @@ macro(curl_prefill_type_size _type _size)
  set(SIZEOF_${_type} ${_size})
  set(SIZEOF_${_type}_CODE "#define SIZEOF_${_type} ${_size}")
 endmacro()
+
+# Create a clang-tidy target for test targets
+macro(curl_clang_tidy_tests _target)
+  if(CURL_CLANG_TIDY)
+
+    # Collect header directories and macro definitions from lib dependencies
+    set(_includes_l "")
+    set(_definitions_l "")
+    get_target_property(_libs ${_target} LINK_LIBRARIES)
+    foreach(_lib IN LISTS _libs)
+      if(TARGET "${_lib}")
+        get_target_property(_val ${_lib} INCLUDE_DIRECTORIES)
+        if(_val)
+          list(APPEND _includes_l ${_val})
+        endif()
+        get_target_property(_val ${_lib} COMPILE_DEFINITIONS)
+        if(_val)
+          list(APPEND _definitions_l ${_val})
+        endif()
+      endif()
+    endforeach()
+
+    # Collect header directories applying to the target
+    get_directory_property(_includes_d INCLUDE_DIRECTORIES)
+    get_target_property(_includes_t ${_target} INCLUDE_DIRECTORIES)
+
+    set(_includes "${_includes_l};${_includes_d};${_includes_t}")
+    list(REMOVE_ITEM _includes "")
+    string(REPLACE ";" ";-I" _includes ";${_includes}")
+
+    # Collect macro definitions applying to the target
+    get_directory_property(_definitions_d COMPILE_DEFINITIONS)
+    get_target_property(_definitions_t ${_target} COMPILE_DEFINITIONS)
+
+    set(_definitions "${_definitions_l};${_definitions_d};${_definitions_t}")
+    list(REMOVE_ITEM _definitions "")
+    string(REPLACE ";" ";-D" _definitions ";${_definitions}")
+    list(SORT _definitions)  # Sort like CMake does
+
+    # Assemble source list
+    set(_sources "")
+    foreach(_source IN ITEMS ${ARGN})
+      if(NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${_source}")  # if not in source tree
+        set(_source "${CMAKE_CURRENT_BINARY_DIR}/${_source}")  # look in the build tree, for generated files, e.g. lib1521.c
+      endif()
+      list(APPEND _sources "${_source}")
+    endforeach()
+
+    add_custom_target("${_target}-clang-tidy" USES_TERMINAL
+      WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
+      COMMAND ${CMAKE_C_CLANG_TIDY} ${_sources} -- ${_includes} ${_definitions}
+      DEPENDS ${_sources})
+    add_dependencies(tests-clang-tidy "${_target}-clang-tidy")
+
+    unset(_includes_d)
+    unset(_includes_t)
+    unset(_includes)
+    unset(_definitions_l)
+    unset(_definitions_d)
+    unset(_definitions_t)
+    unset(_definitions)
+    unset(_sources)
+    unset(_source)
+    unset(_libs)
+    unset(_lib)
+    unset(_val)
+  endif()
+endmacro()
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@ -288,11 +288,9 @@ endif()

 option(CURL_CLANG_TIDY "Run the build through clang-tidy" OFF)
 if(CURL_CLANG_TIDY)
-  # clang-tidy is not looking into #included sources, thus not compatible with
-  # unity builds. Make test bundles compatible by embedding the source code.
-  set(CMAKE_UNITY_BUILD OFF)
-  set(CURL_MK_UNITY_OPTION "--embed")
+  set(CMAKE_UNITY_BUILD OFF)  # clang-tidy is not looking into #included sources, thus not compatible with unity builds.
  set(_tidy_checks "")
+  list(APPEND _tidy_checks "-clang-analyzer-security.insecureAPI.bzero")  # for FD_ZERO() (seen on macOS)
  list(APPEND _tidy_checks "-clang-analyzer-security.insecureAPI.strcpy")
  list(APPEND _tidy_checks "-clang-analyzer-optin.performance.Padding")
  list(APPEND _tidy_checks "-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling")
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@ -156,7 +156,7 @@ endif
 endif

 # disable the tests that are mostly causing false positives
-TIDYFLAGS := -checks=-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling -quiet
+TIDYFLAGS := -checks=-clang-analyzer-security.insecureAPI.bzero,-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling -quiet
 if CURL_WERROR
 TIDYFLAGS += --warnings-as-errors=*
 endif
--- a/scripts/mk-unity.pl
+++ b/scripts/mk-unity.pl
@ -31,16 +31,13 @@ use strict;
 use warnings;

 if(!@ARGV) {
-    die "Usage: $0 [--test <tests>] [--include <include-c-sources>] [--srcdir <srcdir>] [--embed]\n";
+    die "Usage: $0 [--test <tests>] [--include <include-c-sources>]\n";
 }

 my @src;
 my %include;
 my $in_include = 0;
-my $srcdir = "";
-my $in_srcdir = 0;
 my $any_test = 0;
-my $embed = 0;
 foreach my $src (@ARGV) {
    if($src eq "--test") {
        $in_include = 0;
@ -48,16 +45,6 @@ foreach my $src (@ARGV) {
    elsif($src eq "--include") {
        $in_include = 1;
    }
-    elsif($src eq "--embed") {
-        $embed = 1;
-    }
-    elsif($src eq "--srcdir") {
-        $in_srcdir = 1;
-    }
-    elsif($in_srcdir) {
-        $srcdir = $src;
-        $in_srcdir = 0;
-    }
    elsif($in_include) {
        $include{$src} = 1;
        push @src, $src;
@ -78,18 +65,7 @@ my $tlist = "";
 foreach my $src (@src) {
    if($src =~ /([a-z0-9_]+)\.c$/) {
        my $name = $1;
-        if($embed) {
-            my $fn = $src;
-            if($srcdir ne "" && -e "$srcdir/$fn") {
-                $fn = $srcdir . "/" . $fn;
-            }
-            print "/* Embedding: \"$src\" */\n";
-            my $content = do { local $/; open my $fh, '<', $fn or die $!; <$fh> };
-            print $content;
-        }
-        else {
-            print "#include \"$src\"\n";
-        }
+        print "#include \"$src\"\n";
        if(not exists $include{$src}) {  # register test entry function
            $tlist .= "  {\"$name\", test_$name},\n";
        }
--- a/src/Makefile.am
+++ b/src/Makefile.am
@ -80,7 +80,7 @@ else
 curlx_src = $(CURLX_CFILES)
 endif
 curltool_unity.c: $(top_srcdir)/scripts/mk-unity.pl $(CURL_CFILES) $(curl_cfiles_gen) $(curlx_src)
-	@PERL@ $(top_srcdir)/scripts/mk-unity.pl --srcdir $(srcdir) --include $(CURL_CFILES) $(curl_cfiles_gen) $(curlx_src) > curltool_unity.c
+	@PERL@ $(top_srcdir)/scripts/mk-unity.pl --include $(CURL_CFILES) $(curl_cfiles_gen) $(curlx_src) > curltool_unity.c

 nodist_curl_SOURCES = curltool_unity.c
 curl_SOURCES =
@ -205,7 +205,7 @@ endif
 endif

 # disable the tests that are mostly causing false positives
-TIDYFLAGS := -checks=-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling -quiet
+TIDYFLAGS := -checks=-clang-analyzer-security.insecureAPI.bzero,-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling -quiet
 if CURL_WERROR
 TIDYFLAGS += --warnings-as-errors=*
 endif
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@ -34,6 +34,11 @@ if(BUILD_CURL_EXE)
  add_dependencies(testdeps curlinfo)
 endif()

+if(CURL_CLANG_TIDY)
+  add_custom_target(tests-clang-tidy)
+  add_dependencies(testdeps tests-clang-tidy)
+endif()
+
 add_subdirectory(http)
 add_subdirectory(client)
 add_subdirectory(server)
--- a/tests/client/CMakeLists.txt
+++ b/tests/client/CMakeLists.txt
@ -31,8 +31,8 @@ if(LIB_SELECTED STREQUAL LIB_STATIC)
 endif()

 add_custom_command(OUTPUT "${BUNDLE}.c"
-  COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" --include ${CURLX_CFILES} --test ${TESTFILES}
-    ${CURL_MK_UNITY_OPTION} --srcdir "${CMAKE_CURRENT_SOURCE_DIR}" > "${BUNDLE}.c"
+  COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl"
+    --include ${CURLX_CFILES} --test ${TESTFILES} > "${BUNDLE}.c"
  DEPENDS
    "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" "${CMAKE_CURRENT_SOURCE_DIR}/Makefile.inc"
    ${FIRSTFILES} ${CURLX_CFILES} ${TESTFILES}
@ -48,4 +48,6 @@ target_include_directories(${BUNDLE} PRIVATE
  "${CMAKE_CURRENT_SOURCE_DIR}"          # for the generated bundle source to find included test sources
 )
 set_property(TARGET ${BUNDLE} APPEND PROPERTY COMPILE_DEFINITIONS "CURL_NO_OLDIES")
-set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF)
+set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF C_CLANG_TIDY "")
+
+curl_clang_tidy_tests(${BUNDLE} ${FIRSTFILES} ${TESTFILES})
--- a/tests/libtest/CMakeLists.txt
+++ b/tests/libtest/CMakeLists.txt
@ -40,8 +40,8 @@ add_custom_command(OUTPUT "lib1521.c"
 list(APPEND TESTFILES "lib1521.c")

 add_custom_command(OUTPUT "${BUNDLE}.c"
-  COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" --include ${UTILS} ${CURLX_CFILES} --test ${TESTFILES}
-    ${CURL_MK_UNITY_OPTION} --srcdir "${CMAKE_CURRENT_SOURCE_DIR}" > "${BUNDLE}.c"
+  COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl"
+    --include ${UTILS} ${CURLX_CFILES} --test ${TESTFILES} > "${BUNDLE}.c"
  DEPENDS
    "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" "${CMAKE_CURRENT_SOURCE_DIR}/Makefile.inc"
    ${FIRSTFILES} ${UTILS} ${CURLX_CFILES} ${TESTFILES}
@ -59,7 +59,9 @@ target_include_directories(${BUNDLE} PRIVATE
 )
 set_property(TARGET ${BUNDLE} APPEND PROPERTY COMPILE_DEFINITIONS "${CURL_DEBUG_MACROS}")
 set_property(TARGET ${BUNDLE} APPEND PROPERTY COMPILE_DEFINITIONS "CURL_NO_OLDIES" "CURL_DISABLE_DEPRECATION")
-set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF)
+set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF C_CLANG_TIDY "")
+
+curl_clang_tidy_tests(${BUNDLE} ${FIRSTFILES} ${UTILS} ${TESTFILES} ${STUB_GSS})

 if(HAVE_GSSAPI AND UNIX)
  add_library(stubgss SHARED EXCLUDE_FROM_ALL ${STUB_GSS})
--- a/tests/server/CMakeLists.txt
+++ b/tests/server/CMakeLists.txt
@ -27,8 +27,8 @@ curl_transform_makefile_inc("Makefile.inc" "${CMAKE_CURRENT_BINARY_DIR}/Makefile
 include("${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake")

 add_custom_command(OUTPUT "${BUNDLE}.c"
-  COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" --include ${UTILS} ${CURLX_CFILES} --test ${TESTFILES}
-    ${CURL_MK_UNITY_OPTION} --srcdir "${CMAKE_CURRENT_SOURCE_DIR}" > "${BUNDLE}.c"
+  COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl"
+    --include ${UTILS} ${CURLX_CFILES} --test ${TESTFILES} > "${BUNDLE}.c"
  DEPENDS
    "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" "${CMAKE_CURRENT_SOURCE_DIR}/Makefile.inc"
    ${FIRSTFILES} ${UTILS} ${CURLX_CFILES} ${TESTFILES}
@ -54,4 +54,6 @@ set_property(TARGET ${BUNDLE} APPEND PROPERTY COMPILE_DEFINITIONS "CURL_NO_OLDIE
 if(WIN32)
  set_property(TARGET ${BUNDLE} APPEND PROPERTY COMPILE_DEFINITIONS "CURL_STATICLIB")
 endif()
-set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF)
+set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF C_CLANG_TIDY "")
+
+curl_clang_tidy_tests(${BUNDLE} ${FIRSTFILES} ${UTILS} ${TESTFILES})
--- a/tests/server/mqttd.c
+++ b/tests/server/mqttd.c
@ -438,6 +438,7 @@ static curl_socket_t mqttit(curl_socket_t fd)
    logmsg("Out of memory, unable to allocate buffer");
    goto end;
  }
+  memset(buffer, 0, buff_size);

  do {
    unsigned char usr_flag = 0x80;
--- a/tests/server/sockfilt.c
+++ b/tests/server/sockfilt.c
@ -370,7 +370,7 @@ static void lograw(unsigned char *buffer, ssize_t len)
 * *buffer_len is the amount of data in the buffer (output)
 */
 static bool read_data_block(unsigned char *buffer, ssize_t maxlen,
-    ssize_t *buffer_len)
+                            ssize_t *buffer_len)
 {
  if(!read_stdin(buffer, 5))
    return FALSE;
@ -1118,6 +1118,9 @@ static bool juggle(curl_socket_t *sockfdp,
      if(!read_data_block(buffer, sizeof(buffer), &buffer_len))
        return FALSE;

+      if(buffer_len < 0)
+        return FALSE;
+
      if(*mode == PASSIVE_LISTEN) {
        logmsg("*** We are disconnected!");
        if(!disc_handshake())
--- a/tests/tunit/CMakeLists.txt
+++ b/tests/tunit/CMakeLists.txt
@ -27,8 +27,8 @@ curl_transform_makefile_inc("Makefile.inc" "${CMAKE_CURRENT_BINARY_DIR}/Makefile
 include("${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake")

 add_custom_command(OUTPUT "${BUNDLE}.c"
-  COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" --test ${TESTFILES}
-    ${CURL_MK_UNITY_OPTION} --srcdir "${CMAKE_CURRENT_SOURCE_DIR}" > "${BUNDLE}.c"
+  COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl"
+    --test ${TESTFILES} > "${BUNDLE}.c"
  DEPENDS
    "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" "${CMAKE_CURRENT_SOURCE_DIR}/Makefile.inc" ${FIRSTFILES} ${TESTFILES}
  VERBATIM)
@ -47,4 +47,6 @@ target_include_directories(${BUNDLE} PRIVATE
 )
 set_property(TARGET ${BUNDLE} APPEND PROPERTY COMPILE_DEFINITIONS "${CURL_DEBUG_MACROS}")
 set_property(TARGET ${BUNDLE} APPEND PROPERTY COMPILE_DEFINITIONS "CURL_NO_OLDIES" "CURL_DISABLE_DEPRECATION")
-set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF)
+set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF C_CLANG_TIDY "")
+
+curl_clang_tidy_tests(${BUNDLE} ${FIRSTFILES} ${TESTFILES})
--- a/tests/unit/CMakeLists.txt
+++ b/tests/unit/CMakeLists.txt
@ -27,8 +27,8 @@ curl_transform_makefile_inc("Makefile.inc" "${CMAKE_CURRENT_BINARY_DIR}/Makefile
 include("${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake")

 add_custom_command(OUTPUT "${BUNDLE}.c"
-  COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" --test ${TESTFILES}
-    ${CURL_MK_UNITY_OPTION} --srcdir "${CMAKE_CURRENT_SOURCE_DIR}" > "${BUNDLE}.c"
+  COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl"
+    --test ${TESTFILES} > "${BUNDLE}.c"
  DEPENDS
    "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" "${CMAKE_CURRENT_SOURCE_DIR}/Makefile.inc" ${FIRSTFILES} ${TESTFILES}
  VERBATIM)
@ -47,4 +47,6 @@ set_property(TARGET ${BUNDLE} APPEND PROPERTY COMPILE_DEFINITIONS "${CURL_DEBUG_
 set_property(TARGET ${BUNDLE} APPEND PROPERTY COMPILE_DEFINITIONS "CURL_NO_OLDIES" "CURL_DISABLE_DEPRECATION")
 # unit tests are small pretend-libcurl-programs
 set_property(TARGET ${BUNDLE} APPEND PROPERTY COMPILE_DEFINITIONS "BUILDING_LIBCURL")
-set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF)
+set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF C_CLANG_TIDY "")
+
+curl_clang_tidy_tests(${BUNDLE} ${FIRSTFILES} ${TESTFILES})