clang-tidy: add to CI, add cmake support, fix fallouts

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

.github/workflows/linux.yml

@@ -37,6 +37,7 @@ permissions: {}
 
 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
@@ -210,6 +211,12 @@ jobs:
           - 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
@@ -287,7 +294,8 @@ jobs:
           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
@@ -595,7 +603,7 @@ jobs:
           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'
@@ -609,8 +617,9 @@ jobs:
           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 }}
@@ -671,6 +680,7 @@ jobs:
           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

.github/workflows/macos.yml

@@ -131,6 +131,10 @@ jobs:
           - 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
@@ -161,6 +165,8 @@ jobs:
           - { 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' } }
@@ -176,7 +182,9 @@ jobs:
         # 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'
@@ -281,12 +289,14 @@ jobs:
           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 }}'

CMakeLists.txt

@@ -247,6 +247,25 @@ if(ENABLE_CURLDEBUG)
   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")

configure.ac

@@ -530,6 +530,7 @@ if test X"$want_werror" = Xyes; then
   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

docs/INSTALL-CMAKE.md

@@ -149,6 +149,8 @@ assumes that CMake generates `Makefile`:
 - `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`
@@ -303,6 +305,7 @@ Details via CMake
 
 ## 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.

docs/examples/anyauthput.c

@@ -100,6 +100,9 @@ int main(int argc, char **argv)
 
   /* 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 */

docs/examples/fileupload.c

@@ -51,8 +51,10 @@ int main(void)
     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) {

docs/examples/ftpgetresp.c

@@ -49,9 +49,15 @@ int main(void)
 
   /* 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) {

docs/examples/ftpupload.c

@@ -91,6 +91,8 @@ int main(void)
 
   /* 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);

docs/examples/httpput.c

@@ -86,6 +86,8 @@ int main(int argc, char **argv)
      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);

docs/examples/simplessl.c

@@ -74,6 +74,8 @@ int main(void)
 #endif
 
   headerfile = fopen(pHeaderFile, "wb");
+  if(!headerfile)
+    return 1;
 
   curl_global_init(CURL_GLOBAL_DEFAULT);
 
@@ -146,5 +148,7 @@ int main(void)
 
   curl_global_cleanup();
 
+  fclose(headerfile);
+
   return 0;
 }

lib/Makefile.am

@@ -155,12 +155,15 @@ all-local: checksrc
 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

lib/cf-h2-proxy.c

@@ -865,7 +865,9 @@ static int tunnel_recv_callback(nghttp2_session *session, uint8_t flags,
   if(nwritten < 0) {
     if(result != CURLE_AGAIN)
       return NGHTTP2_ERR_CALLBACK_FAILURE;
+#ifdef DEBUGBUILD
     nwritten = 0;
+#endif
   }
   DEBUGASSERT((size_t)nwritten == len);
   return 0;

lib/cf-socket.c

@@ -1850,7 +1850,7 @@ static CURLcode cf_udp_setup_quic(struct Curl_cfilter *cf,
   /* 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);

lib/ftp.c

@@ -4253,7 +4253,7 @@ CURLcode ftp_parse_url_path(struct Curl_easy *data)
       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;
       }

lib/http2.c

@@ -2337,7 +2337,7 @@ static ssize_t cf_h2_send(struct Curl_cfilter *cf, struct Curl_easy *data,
   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 {

lib/mprintf.c

@@ -1014,7 +1014,7 @@ number:
       /* 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

src/Makefile.am

@@ -190,12 +190,15 @@ all-local: checksrc
 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)

src/tool_writeout.c

@@ -412,8 +412,8 @@ static int writeString(FILE *stream, const struct writeoutvar *wovar,
     }
   }
 
-  if(valid) {
-    DEBUGASSERT(strinfo);
+  DEBUGASSERT(!valid || strinfo);
+  if(valid && strinfo) {
     if(use_json) {
       fprintf(stream, "\"%s\":", wovar->name);
       jsonWriteString(stream, strinfo, FALSE);

tests/http/clients/hx-download.c

@@ -313,12 +313,14 @@ int main(int argc, char *argv[])
   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;
@@ -350,6 +352,7 @@ int main(int argc, char *argv[])
       pause_offset = (size_t)strtol(optarg, NULL, 10);
       break;
     case 'r':
+      free(resolve);
       resolve = strdup(optarg);
       break;
     case 'V': {
@@ -361,13 +364,15 @@ int main(int argc, char *argv[])
         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;
@@ -378,7 +383,8 @@ int main(int argc, char *argv[])
 
   if(argc != 1) {
     usage("not enough arguments");
-    return 2;
+    result = 2;
+    goto cleanup;
   }
   url = argv[0];
 
@@ -388,7 +394,8 @@ int main(int argc, char *argv[])
   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);
@@ -400,7 +407,8 @@ int main(int argc, char *argv[])
   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();
@@ -425,7 +433,8 @@ int main(int argc, char *argv[])
        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;
@@ -468,7 +477,6 @@ int main(int argc, char *argv[])
         }
       }
 
-
       /* nothing happening, maintenance */
       if(abort_paused) {
         /* abort paused transfers */
@@ -505,7 +513,8 @@ int main(int argc, char *argv[])
                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;
@@ -539,9 +548,10 @@ int main(int argc, char *argv[])
 
   curl_share_cleanup(share);
   curl_slist_free_all(host);
+cleanup:
   free(resolve);
 
-  return 0;
+  return result;
 #else
   (void)argc;
   (void)argv;

tests/server/getpart.c

@@ -349,9 +349,6 @@ int getpart(char **outbuf, size_t *outlen,
         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);
@@ -368,9 +365,6 @@ int getpart(char **outbuf, size_t *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);
@@ -386,11 +380,8 @@ int getpart(char **outbuf, size_t *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;
-        }
       }
 
     }

tests/server/mqttd.c

@@ -542,12 +542,14 @@ static curl_socket_t mqttit(curl_socket_t fd)
       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) {

tests/server/rtspd.c

@@ -641,12 +641,10 @@ static void storerequest(char *reqbuf, size_t totalsize)
 
 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 */
@@ -976,12 +974,10 @@ static int send_doc(curl_socket_t sock, struct httprequest *req)
     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);

tests/server/socksd.c

@@ -1167,7 +1167,7 @@ socks5_cleanup:
     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));
   }

tests/server/sws.c

@@ -832,12 +832,10 @@ static void storerequest(const char *reqbuf, size_t totalsize)
 
 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)
@@ -1217,12 +1215,10 @@ retry:
     }
   } 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);
@@ -2527,7 +2523,7 @@ sws_cleanup:
     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));
   }

tests/server/tftpd.c

@@ -425,9 +425,9 @@ static int writeit(struct testcase *test, struct tftphdr * volatile *dpp,
   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 */
 }
 
@@ -448,7 +448,7 @@ static ssize_t write_behind(struct testcase *test, int convert)
   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) {
@@ -464,6 +464,9 @@ static ssize_t write_behind(struct testcase *test, int convert)
       return -1; /* failure! */
     }
   }
+  else if(test->ofile <= 0) {
+    return -1; /* failure! */
+  }
 
   count = b->counter;             /* remember byte count */
   b->counter = BF_FREE;           /* reset flag */
@@ -952,7 +955,7 @@ static int do_tftp(struct testcase *test, struct tftphdr *tp, ssize_t size)
       break;
   } while(1);
 
-  if(*cp) {
+  if(*cp || !mode) {
     nak(EBADOP);
     fclose(server);
     return 3;

tests/server/util.c

@@ -341,12 +341,10 @@ void set_advisor_read_lock(const char *filename)
     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)