build:
- autotools: fix to build generated sources for the `tidy` target.
- autotools: allow passing custom clang-tidy options via
`CURL_CLANG_TIDYFLAGS` env.
- cmake: add `CURL_CLANG_TIDY` option to configure for `clang-tidy`.
Also add:
- `CLANG_TIDY` variable to customize the `clang-tidy` tool.
- `CURL_CLANG_TIDYFLAGS` to pass custom options to `clang-tidy`.
- apply `--enable-werror` and `-DCURL_WERROR=ON` to `clang-tidy`.
CI/GHA:
- add clang-tidy job for Linux, using autotools and clang-tidy v18.
This one needs to disable `clang-analyzer-valist.Uninitialized`
to avoid false positives:
https://github.com/llvm/llvm-project/issues/40656
Duration: 5.5 minutes
- add clang-tidy job for macOS, using cmake and clang-tidy v19.
This one also covers tests and examples, and doesn't hit the false
positives seen with llvm v18 and earlier.
Duration: 4.5 minutes
- Linux/macOS: skip installing test dependencies when not building or
running tests.
fix fallouts reported by `clang-tidy`:
- lib:
- cf-h2-proxy: unused assignment in non-debug builds.
- cf-socket: silence warning.
FIXME: https://github.com/curl/curl/pull/15825#issuecomment-
2561867769
- ftp: NULL passed to `strncmp()`.
- http2: NULL-ptr deref.
- mprintf: silence warning.
- src/tool_writeout: NULL passed to `fputs()`.
- examples:
- invalid file pointers.
- missing `fclose()`.
- tests:
- http/clients/hx-download: memory leaks on error.
- http/clients/hx-download: memory leak on repeat `-r` option.
- server: double `fclose()`.
https://www.man7.org/linux/man-pages/man3/fclose.3.html
- server: invalid file pointer/handle.
- server/getpart: unused assignments.
- server/mqttd: leak on failed `realloc()`.
- server/tftpd: NULL passed to `strcmp()`.
Closes #15825
env:
MAKEFLAGS: -j 5
+ CURL_CLANG_TIDYFLAGS: '-checks=-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,-clang-analyzer-valist.Uninitialized'
# unhandled
bearssl-version: 0.6
# renovate: datasource=github-tags depName=libressl-portable/portable versioning=semver registryUrl=https://github.com
- name: '!ssl !http !smtp !imap'
configure: --without-ssl --enable-debug --disable-http --disable-smtp --disable-imap --disable-unity
+ - name: clang-tidy
+ install_packages: clang-tidy libssl-dev libssh2-1-dev
+ install_steps: skipall
+ configure: --with-openssl --with-libssh2
+ make-custom-target: tidy
+
- name: scanbuild
install_packages: clang-tools clang libssl-dev libssh2-1-dev
install_steps: skipall
sudo rm -f /etc/apt/sources.list.d/microsoft-prod.list
sudo apt-get update -y
sudo apt-get install -y --no-install-suggests --no-install-recommends \
- libtool autoconf automake pkgconf ninja-build stunnel4 \
+ libtool autoconf automake pkgconf ninja-build \
+ ${{ matrix.build.install_steps != 'skipall' && matrix.build.install_steps != 'skiprun' && 'stunnel4' || '' }} \
libpsl-dev libbrotli-dev libzstd-dev \
${{ matrix.build.install_packages }}
python3 -m venv $HOME/venv
if [ -n '${{ matrix.build.generate }}' ]; then
${{ matrix.build.make-prefix }} cmake --build . --verbose
else
- ${{ matrix.build.make-prefix }} make V=1
+ ${{ matrix.build.make-prefix }} make V=1 ${{ matrix.build.make-custom-target }}
fi
- name: 'single-use function check'
fi
./scripts/singleuse.pl --unit ${libcurla}
- - run: ./src/curl -V
- name: 'check curl -V output'
+ - name: 'check curl -V output'
+ if: ${{ matrix.build.make-custom-target != 'tidy' }}
+ run: ./src/curl -V
- run: cmake --install . --prefix $HOME/curl --strip
if: ${{ matrix.build.generate }}
fi
- name: 'build examples'
+ if: ${{ matrix.build.make-custom-target != 'tidy' }}
run: |
if [ -n '${{ matrix.build.generate }}' ]; then
${{ matrix.build.make-prefix }} cmake --build . --verbose --target curl-examples
- name: 'OpenSSL gsasl rtmp AppleIDN'
install: gsasl rtmpdump
generate: -DOPENSSL_ROOT_DIR=$(brew --prefix openssl) -DCURL_USE_GSASL=ON -DUSE_LIBRTMP=ON -DUSE_APPLE_IDN=ON
+ - name: 'OpenSSL AppleIDN clang-tidy +examples'
+ install: llvm
+ generate: -DOPENSSL_ROOT_DIR=$(brew --prefix openssl) -DUSE_APPLE_IDN=ON -DCURL_CLANG_TIDY=ON -DCLANG_TIDY=$(brew --prefix llvm)/bin/clang-tidy
+ clang-tidy: true
- name: 'OpenSSL +static libssh +examples'
install: libssh
generate: -DOPENSSL_ROOT_DIR=$(brew --prefix openssl) -DBUILD_STATIC_LIBS=ON -DCURL_USE_LIBSSH2=OFF -DCURL_USE_LIBSSH=ON
- { compiler: llvm@15, build: { macos-version-min: '10.15' } }
- { compiler: llvm@15, build: { torture: true } }
- { compiler: gcc-12, build: { torture: true } }
+ - { compiler: llvm@15, build: { clang-tidy: true } }
+ - { compiler: gcc-12, build: { clang-tidy: true } }
# opt out jobs from combinations that have the compiler set manually
- { compiler: llvm@15, build: { compiler: 'clang' } }
- { compiler: llvm@15, build: { compiler: 'gcc-12' } }
# https://github.com/curl/curl/runs/4095721123?check_suite_focus=true
run: |
echo ${{ matrix.build.generate && 'ninja' || 'automake libtool' }} \
- pkgconf libpsl libssh2 nghttp2 stunnel ${{ matrix.build.install }} | xargs -Ix -n1 echo brew '"x"' > /tmp/Brewfile
+ pkgconf libpsl libssh2 \
+ ${{ !matrix.build.clang-tidy && 'nghttp2 stunnel' || '' }} \
+ ${{ matrix.build.install }} | xargs -Ix -n1 echo brew '"x"' > /tmp/Brewfile
while [[ $? == 0 ]]; do for i in 1 2 3; do brew update && brew bundle install --no-lock --file /tmp/Brewfile && break 2 || { echo Error: wait to try again; sleep 10; } done; false Too many retries; done
- name: 'brew unlink openssl'
fi
- name: 'install test prereqs'
+ if: ${{ !matrix.build.clang-tidy }}
run: |
python3 -m venv $HOME/venv
source $HOME/venv/bin/activate
python3 -m pip install -r tests/requirements.txt
- name: 'run tests'
+ if: ${{ !matrix.build.clang-tidy }}
timeout-minutes: ${{ matrix.build.torture && 20 || 10 }}
run: |
export TFLAGS='-j20 ${{ matrix.build.tflags }}'
set_property(DIRECTORY APPEND PROPERTY COMPILE_DEFINITIONS "CURLDEBUG")
endif()
+option(CURL_CLANG_TIDY "Run the build through clang-tidy" OFF)
+if(CURL_CLANG_TIDY)
+ set(CMAKE_UNITY_BUILD OFF)
+ set(_tidy_checks "")
+ 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")
+ 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)
+ 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})
+ endif()
+endif()
+
# For debug libs and exes, add "-d" postfix
if(NOT DEFINED CMAKE_DEBUG_POSTFIX)
set(CMAKE_DEBUG_POSTFIX "-d")
fi
fi
AC_SUBST(CURL_CFLAG_EXTRAS)
+AM_CONDITIONAL(CURL_WERROR, test X"$want_werror" = Xyes)
CURL_CHECK_COMPILER_HALT_ON_ERROR
CURL_CHECK_COMPILER_ARRAY_SIZE_NEGATIVE
- `BUILD_STATIC_CURL`: Build curl executable with static libcurl. Default: `OFF`
- `BUILD_STATIC_LIBS`: Build static libraries. Default: `OFF`
- `BUILD_TESTING`: Build tests. Default: `ON`
+- `CURL_CLANG_TIDY`: Run the build through `clang-tidy`. Default: `OFF`
+- `CURL_CLANG_TIDYFLAGS`: Custom options to pass to `clang-tidy`. Default: (empty)
- `CURL_DEFAULT_SSL_BACKEND`: Override default TLS backend in MultiSSL builds.
Accepted values in order of default priority:
`wolfssl`, `gnutls`, `mbedtls`, `openssl`, `secure-transport`, `schannel`, `bearssl`, `rustls`
## Dependency options
+- `CLANG_TIDY`: `clang-tidy` tool used with `CURL_CLANG_TIDY=ON`. Default: `clang-tidy`
- `PERL_EXECUTABLE` Perl binary used throughout the build and tests.
- `AMISSL_INCLUDE_DIR`: The AmiSSL include directory.
- `AMISSL_STUBS_LIBRARY`: Path to `amisslstubs` library.
/* get the file size of the local file */
fp = fopen(file, "rb");
+ if(!fp)
+ return 2;
+
fstat(fileno(fp), &file_info);
/* In Windows, this inits the Winsock stuff */
return 1; /* cannot continue */
/* to get the file size */
- if(fstat(fileno(fd), &file_info) != 0)
+ if(fstat(fileno(fd), &file_info) != 0) {
+ fclose(fd);
return 1; /* cannot continue */
+ }
curl = curl_easy_init();
if(curl) {
/* local filename to store the file as */
ftpfile = fopen(FTPBODY, "wb"); /* b is binary, needed on Windows */
+ if(!ftpfile)
+ return 1;
/* local filename to store the FTP server's response lines in */
respfile = fopen(FTPHEADERS, "wb"); /* b is binary, needed on Windows */
+ if(!respfile) {
+ fclose(ftpfile);
+ return 1;
+ }
curl = curl_easy_init();
if(curl) {
/* get a FILE * of the same file */
hd_src = fopen(LOCAL_FILE, "rb");
+ if(!hd_src)
+ return 2;
/* In Windows, this inits the Winsock stuff */
curl_global_init(CURL_GLOBAL_ALL);
fdopen() from the previous descriptor, but hey this is just
an example! */
hd_src = fopen(file, "rb");
+ if(!hd_src)
+ return 2;
/* In Windows, this inits the Winsock stuff */
curl_global_init(CURL_GLOBAL_ALL);
#endif
headerfile = fopen(pHeaderFile, "wb");
+ if(!headerfile)
+ return 1;
curl_global_init(CURL_GLOBAL_DEFAULT);
curl_global_cleanup();
+ fclose(headerfile);
+
return 0;
}
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.strcpy,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling -quiet
+if CURL_WERROR
+TIDYFLAGS += --warnings-as-errors=*
+endif
-TIDY:=clang-tidy
+TIDY := clang-tidy
tidy:
- $(TIDY) $(CSOURCES) $(TIDYFLAGS) -- $(AM_CPPFLAGS) $(CPPFLAGS) -DHAVE_CONFIG_H
+ $(TIDY) $(CSOURCES) $(TIDYFLAGS) $(CURL_CLANG_TIDYFLAGS) -- $(AM_CPPFLAGS) $(CPPFLAGS) -DHAVE_CONFIG_H
optiontable:
perl optiontable.pl < $(top_srcdir)/include/curl/curl.h > easyoptions.c
if(nwritten < 0) {
if(result != CURLE_AGAIN)
return NGHTTP2_ERR_CALLBACK_FAILURE;
+#ifdef DEBUGBUILD
nwritten = 0;
+#endif
}
DEBUGASSERT((size_t)nwritten == len);
return 0;
/* QUIC needs a connected socket, nonblocking */
DEBUGASSERT(ctx->sock != CURL_SOCKET_BAD);
- rc = connect(ctx->sock, &ctx->addr.curl_sa_addr,
+ rc = connect(ctx->sock, &ctx->addr.curl_sa_addr, /* NOLINT FIXME */
(curl_socklen_t)ctx->addr.addrlen);
if(-1 == rc) {
return socket_connect_result(data, ctx->ip.remote_ip, SOCKERRNO);
else
n -= ftpc->file ? strlen(ftpc->file) : 0;
- if((strlen(oldPath) == n) && !strncmp(rawPath, oldPath, n)) {
+ if((strlen(oldPath) == n) && rawPath && !strncmp(rawPath, oldPath, n)) {
infof(data, "Request has same path as previous transfer");
ftpc->cwddone = TRUE;
}
if(should_close_session(ctx)) {
/* nghttp2 thinks this session is done. If the stream has not been
* closed, this is an error state for out transfer */
- if(stream->closed) {
+ if(stream && stream->closed) {
nwritten = http2_handle_stream_close(cf, data, stream, err);
}
else {
/* NOTE NOTE NOTE!! Not all sprintf implementations return number of
output characters */
#ifdef HAVE_SNPRINTF
- (snprintf)(work, sizeof(work), formatbuf, iptr->val.dnum);
+ (snprintf)(work, sizeof(work), formatbuf, iptr->val.dnum); /* NOLINT */
#else
(sprintf)(work, formatbuf, iptr->val.dnum);
#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.strcpy,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling -quiet
+if CURL_WERROR
+TIDYFLAGS += --warnings-as-errors=*
+endif
-TIDY:=clang-tidy
+TIDY := clang-tidy
-tidy:
- $(TIDY) $(CURL_CFILES) $(TIDYFLAGS) -- $(curl_CPPFLAGS) $(CPPFLAGS) $(AM_CPPFLAGS) -DHAVE_CONFIG_H
+tidy: $(HUGE) $(CA_EMBED_CSOURCE)
+ $(TIDY) $(CURL_CFILES) $(TIDYFLAGS) $(CURL_CLANG_TIDYFLAGS) -- $(curl_CPPFLAGS) $(CPPFLAGS) $(AM_CPPFLAGS) -DHAVE_CONFIG_H
listhelp:
(cd $(top_srcdir)/docs/cmdline-opts && make listhelp)
}
}
- if(valid) {
- DEBUGASSERT(strinfo);
+ DEBUGASSERT(!valid || strinfo);
+ if(valid && strinfo) {
if(use_json) {
fprintf(stream, "\"%s\":", wovar->name);
jsonWriteString(stream, strinfo, FALSE);
char *resolve = NULL;
size_t max_host_conns = 0;
int fresh_connect = 0;
+ int result = 0;
while((ch = getopt(argc, argv, "aefhm:n:xA:F:M:P:r:V:")) != -1) {
switch(ch) {
case 'h':
usage(NULL);
- return 2;
+ result = 2;
+ goto cleanup;
case 'a':
abort_paused = 1;
break;
pause_offset = (size_t)strtol(optarg, NULL, 10);
break;
case 'r':
+ free(resolve);
resolve = strdup(optarg);
break;
case 'V': {
http_version = CURL_HTTP_VERSION_3ONLY;
else {
usage("invalid http version");
- return 1;
+ result = 1;
+ goto cleanup;
}
break;
}
default:
- usage("invalid option");
- return 1;
+ usage("invalid option");
+ result = 1;
+ goto cleanup;
}
}
argc -= optind;
if(argc != 1) {
usage("not enough arguments");
- return 2;
+ result = 2;
+ goto cleanup;
}
url = argv[0];
share = curl_share_init();
if(!share) {
fprintf(stderr, "error allocating share\n");
- return 1;
+ result = 1;
+ goto cleanup;
}
curl_share_setopt(share, CURLSHOPT_SHARE, CURL_LOCK_DATA_COOKIE);
curl_share_setopt(share, CURLSHOPT_SHARE, CURL_LOCK_DATA_DNS);
transfers = calloc(transfer_count, sizeof(*transfers));
if(!transfers) {
fprintf(stderr, "error allocating transfer structs\n");
- return 1;
+ result = 1;
+ goto cleanup;
}
multi_handle = curl_multi_init();
setup(t->easy, url, t, http_version, host, share, use_earlydata,
fresh_connect)) {
fprintf(stderr, "[t-%d] FAILED setup\n", (int)i);
- return 1;
+ result = 1;
+ goto cleanup;
}
curl_multi_add_handle(multi_handle, t->easy);
t->started = 1;
}
}
-
/* nothing happening, maintenance */
if(abort_paused) {
/* abort paused transfers */
setup(t->easy, url, t, http_version, host, share,
use_earlydata, fresh_connect)) {
fprintf(stderr, "[t-%d] FAILED setup\n", (int)i);
- return 1;
+ result = 1;
+ goto cleanup;
}
curl_multi_add_handle(multi_handle, t->easy);
t->started = 1;
curl_share_cleanup(share);
curl_slist_free_all(host);
+cleanup:
free(resolve);
- return 0;
+ return result;
#else
(void)argc;
(void)argv;
state = STATE_INMAIN;
csub[0] = '\0';
if(in_wanted_part) {
- /* end of wanted part */
- in_wanted_part = 0;
-
/* Do we need to base64 decode the data? */
if(base64) {
error = decodedata(outbuf, outlen);
state = STATE_OUTER;
cmain[0] = '\0';
if(in_wanted_part) {
- /* end of wanted part */
- in_wanted_part = 0;
-
/* Do we need to base64 decode the data? */
if(base64) {
error = decodedata(outbuf, outlen);
/* end of outermost file section */
state = STATE_OUTSIDE;
couter[0] = '\0';
- if(in_wanted_part) {
- /* end of wanted part */
- in_wanted_part = 0;
+ if(in_wanted_part)
break;
- }
}
}
break;
if(remaining_length >= buff_size) {
+ unsigned char *newbuffer;
buff_size = remaining_length;
- buffer = realloc(buffer, buff_size);
- if(!buffer) {
+ newbuffer = realloc(buffer, buff_size);
+ if(!newbuffer) {
logmsg("Failed realloc of size %zu", buff_size);
goto end;
}
+ buffer = newbuffer;
}
if(remaining_length) {
storerequest_cleanup:
- do {
- res = fclose(dump);
- } while(res && ((error = errno) == EINTR));
+ res = fclose(dump);
if(res)
logmsg("Error closing file %s error: %d %s",
- dumpfile, error, strerror(error));
+ dumpfile, errno, strerror(errno));
}
/* return 0 on success, non-zero on failure */
req->rtp_buffersize = 0;
}
- do {
- res = fclose(dump);
- } while(res && ((error = errno) == EINTR));
+ res = fclose(dump);
if(res)
logmsg("Error closing file %s error: %d %s",
- responsedump, error, strerror(error));
+ responsedump, errno, strerror(errno));
if(got_exit_signal) {
free(ptr);
sclose(sock);
#ifdef USE_UNIX_SOCKETS
- if(unlink_socket && socket_domain == AF_UNIX) {
+ if(unlink_socket && socket_domain == AF_UNIX && unix_socket) {
error = unlink(unix_socket);
logmsg("unlink(%s) = %d (%s)", unix_socket, error, strerror(error));
}
storerequest_cleanup:
- do {
- res = fclose(dump);
- } while(res && ((error = errno) == EINTR));
+ res = fclose(dump);
if(res)
logmsg("Error closing file %s error: %d %s",
- dumpfile, error, strerror(error));
+ dumpfile, errno, strerror(errno));
}
static void init_httprequest(struct httprequest *req)
}
} while((count > 0) && !got_exit_signal);
- do {
- res = fclose(dump);
- } while(res && ((error = errno) == EINTR));
+ res = fclose(dump);
if(res)
logmsg("Error closing file %s error: %d %s",
- responsedump, error, strerror(error));
+ responsedump, errno, strerror(errno));
if(got_exit_signal) {
free(ptr);
sclose(sock);
#ifdef USE_UNIX_SOCKETS
- if(unlink_socket && socket_domain == AF_UNIX) {
+ if(unlink_socket && socket_domain == AF_UNIX && unix_socket) {
rc = unlink(unix_socket);
logmsg("unlink(%s) = %d (%s)", unix_socket, rc, strerror(rc));
}
bfs[current].counter = ct; /* set size of data to write */
current = !current; /* switch to other buffer */
if(bfs[current].counter != BF_FREE) /* if not free */
- write_behind(test, convert); /* flush it */
+ write_behind(test, convert); /* flush it */
bfs[current].counter = BF_ALLOC; /* mark as alloc'd */
- *dpp = &bfs[current].buf.hdr;
+ *dpp = &bfs[current].buf.hdr;
return ct; /* this is a lie of course */
}
struct tftphdr *dp;
b = &bfs[nextone];
- if(b->counter < -1) /* anything to flush? */
+ if(b->counter < -1) /* anything to flush? */
return 0; /* just nop if nothing to do */
if(!test->ofile) {
return -1; /* failure! */
}
}
+ else if(test->ofile <= 0) {
+ return -1; /* failure! */
+ }
count = b->counter; /* remember byte count */
b->counter = BF_FREE; /* reset flag */
break;
} while(1);
- if(*cp) {
+ if(*cp || !mode) {
nak(EBADOP);
fclose(server);
return 3;
return;
}
- do {
- res = fclose(lockfile);
- } while(res && ((error = errno) == EINTR));
+ res = fclose(lockfile);
if(res)
logmsg("Error closing lock file %s error: %d %s",
- filename, error, strerror(error));
+ filename, errno, strerror(errno));
}
void clear_advisor_read_lock(const char *filename)