Merge pull request #1677 from advancedtelematic/fix/clang-tidy-8

Upgrade to clang-tidy and clang-format to version 8.

.clang-tidy

@@ -3,14 +3,14 @@ Checks:          >-
   clang-analyzer-*,
   performance-*,
   bugprone-*,
-  readability-*,-readability-else-after-return,-readability-identifier-naming,-readability-container-size-empty,-readability-redundant-declaration,
+  readability-*,-readability-else-after-return,-readability-identifier-naming,-readability-container-size-empty,-readability-redundant-declaration,-readability-magic-numbers,
   modernize-*,-modernize-avoid-bind,-modernize-loop-convert,-modernize-use-using,-modernize-deprecated-headers,-modernize-pass-by-value,
   llvm-namespace-comment,
   cppcoreguidelines-pro-*,-cppcoreguidelines-pro-type-reinterpret-cast,-cppcoreguidelines-pro-type-vararg,-cppcoreguidelines-pro-type-const-cast,
   -cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-bounds-constant-array-index,cert-*,
   -cert-err58-cpp,-cert-env33-c,-cert-err09-cpp,-cert-err60-cpp,-cert-err61-cpp, -cert-err61-cpp,
   google-*,-google-runtime-references,-google-runtime-member-string-references,-google-explicit-constructor,-google-readability-todo,
-  misc-*, -misc-unused-parameters, -misc-macro-parentheses, -misc-throw-by-value-catch-by-reference, -misc-suspicious-enum-usage
+  misc-*, -misc-unused-parameters, -misc-macro-parentheses, -misc-throw-by-value-catch-by-reference, -misc-suspicious-enum-usage, -misc-non-private-member-variables-in-classes
 
 WarningsAsErrors: '*'
 AnalyzeTemporaryDtors: false

CMakeLists.txt

@@ -232,8 +232,8 @@ endif()
 add_custom_target(build_tests)
 
 # clang-check and clang-format
-find_program(CLANG_FORMAT NAMES clang-format-6.0)
-find_program(CLANG_TIDY NAMES clang-tidy-6.0)
+find_program(CLANG_FORMAT NAMES clang-format-8)
+find_program(CLANG_TIDY NAMES clang-tidy-8)
 
 if(CLANG_FORMAT)
     function(aktualizr_clang_format)
@@ -256,7 +256,7 @@ if(CLANG_FORMAT)
         endforeach()
     endfunction()
 else()
-    message(WARNING "clang-format-6.0 not found, skipping")
+    message(WARNING "clang-format-8 not found, skipping")
     function(aktualizr_clang_format)
     endfunction()
 endif()
@@ -267,22 +267,21 @@ if(CLANG_TIDY)
     function(aktualizr_clang_tidy)
         file(RELATIVE_PATH SUBDIR ${CMAKE_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR})
         foreach(FILE ${ARGN})
+            # Skip header files, since they are ignored. Headers are checked if
+            # they are included by a checked source file.
             if(${FILE} MATCHES "\\.h")
-                # do not run clang-tidy directly on header files, since it
-                # ignores them. headers will be checked if they are included by
-                # a checked source file
                 continue()
             endif()
             string(REPLACE "/" "_" TARGETNAME "aktualizr_clang_tidy-${SUBDIR}-${FILE}")
             add_custom_target(${TARGETNAME}
-                COMMAND ${CLANG_TIDY} -quiet -header-filter=\(${CMAKE_SOURCE_DIR}|\\.\\.\)/src/.* --extra-arg-before=-Wno-unknown-warning-option -format-style=file -p ${CMAKE_BINARY_DIR} ${FILE}
+                COMMAND ${PROJECT_SOURCE_DIR}/scripts/clang-tidy-wrapper.sh ${CLANG_TIDY} ${CMAKE_BINARY_DIR} ${CMAKE_SOURCE_DIR} ${FILE}
                 WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
                 VERBATIM)
             add_dependencies(clang-tidy ${TARGETNAME})
         endforeach()
     endfunction()
 else()
-    message(WARNING "clang-tidy-6.0 not found, skipping")
+    message(WARNING "clang-tidy-8 not found, skipping")
     function(aktualizr_clang_tidy)
     endfunction()
 endif()

README.adoc

@@ -56,7 +56,7 @@ The default versions packaged in recent Debian/Ubuntu releases are generally new
 Additional packages are used for non-essential components:
 
 * To build the test suite, you will need `net-tools python3-dev python3-openssl python3-venv sqlite3 valgrind`.
-* To run the linting tools, you will need `clang clang-format-6.0 clang-tidy-6.0`.
+* To run the linting tools, you will need `clang clang-format-8 clang-tidy-8`.
 * To build additional documentation, you will need `doxygen graphviz`.
 * To build with code coverage, you will need `lcov`.
 

docker/Dockerfile.debian.testing

@@ -3,6 +3,8 @@ LABEL Description="Aktualizr testing dockerfile for Debian Unstable + static che
 
 ENV DEBIAN_FRONTEND noninteractive
 
+RUN echo "deb http://deb.debian.org/debian  buster-backports main" > /etc/apt/sources.list.d/buster-backports.list
+
 # It is important to run these in the same RUN command, because otherwise
 # Docker layer caching breaks us
 RUN apt-get update && apt-get -y install --no-install-suggests --no-install-recommends \
@@ -12,10 +14,10 @@ RUN apt-get update && apt-get -y install --no-install-suggests --no-install-reco
   automake \
   bison \
   ccache \
-  clang-6.0 \
-  clang-tidy-6.0 \
-  clang-tools-6.0 \
-  clang-format-6.0 \
+  clang-8 \
+  clang-tidy-8 \
+  clang-tools-8 \
+  clang-format-8 \
   cmake \
   curl \
   doxygen \
@@ -65,8 +67,8 @@ RUN apt-get update && apt-get -y install --no-install-suggests --no-install-reco
   xsltproc \
   zip
 
-RUN ln -s clang-6.0 /usr/bin/clang && \
-    ln -s clang++-6.0 /usr/bin/clang++
+RUN ln -s clang-8 /usr/bin/clang && \
+    ln -s clang++-8 /usr/bin/clang++
 
 WORKDIR /ostree
 RUN git init && git remote add origin https://github.com/ostreedev/ostree

docker/Dockerfile.ubuntu.bionic

@@ -13,7 +13,7 @@ RUN apt-get update && apt-get -y install --no-install-suggests --no-install-reco
   awscli \
   bison \
   ccache \
-  clang-format-6.0 \
+  clang-format-8 \
   cmake \
   curl \
   e2fslibs-dev \

docs/ota-client-guide/modules/ROOT/pages/install-garage-sign-deploy.adoc

@@ -45,11 +45,10 @@ If you're using another version of Ubuntu, or another Debian-based distribution
 [subs="attributes"]
 ----
 git clone --branch {aktualizr-version} --recursive https://github.com/advancedtelematic/aktualizr
-sudo apt install asn1c build-essential clang clang-check-3.8 clang-format-3.8 clang-tidy-3.8 cmake curl \
+sudo apt install asn1c build-essential clang clang-format-8 clang-tidy-8 cmake curl \
   doxygen graphviz lcov libarchive-dev libboost-dev libboost-filesystem-dev libboost-log-dev \
-  libboost-program-options-dev libboost-serialization-dev libboost-iostreams-dev libcurl4-openssl-dev \
-  libdpkg-dev libostree-dev libp11-2 libp11-dev libpthread-stubs0-dev libsodium-dev libsqlite3-dev \
-  libssl-dev libsystemd-dev
+  libboost-program-options-dev libcurl4-openssl-dev libostree-dev libp11-2 libp11-dev \
+  libpthread-stubs0-dev libsodium-dev libsqlite3-dev libssl-dev libsystemd-dev
 cd aktualizr
 mkdir build
 cd build

docs/ota-client-guide/modules/ROOT/pages/simulate-device-basic.adoc

@@ -91,7 +91,7 @@ Build from source::
 Modify as needed for your distro. See https://github.com/advancedtelematic/aktualizr for further instructions
 [source,sh,subs="attributes"]
 ----
-sudo apt install asn1c build-essential clang clang-format-6.0 clang-tidy-6.0 \
+sudo apt install asn1c build-essential clang clang-format-8 clang-tidy-8 \
 cmake curl doxygen graphviz lcov libarchive-dev libboost-dev \
 libboost-filesystem-dev libboost-log-dev libboost-program-options-dev \
 libcurl4-openssl-dev libostree-dev libp11-3 libp11-dev libpthread-stubs0-dev \

scripts/clang-tidy-wrapper.sh

@@ -0,0 +1,21 @@
+#! /bin/bash
+
+set -euo pipefail
+
+CLANG_TIDY="${1}"
+CMAKE_BINARY_DIR="${2}"
+CMAKE_SOURCE_DIR="${3}"
+FILE="${4}"
+
+if [[ ! -e "${CMAKE_BINARY_DIR}/compile_commands.json" ]]; then
+  echo "compile_commands.json not found!"
+  exit 1
+fi
+
+# Filter out anything that we aren't trying to compile. Older clang-tidy
+# versions (<=6) skipped them automatically.
+if grep "${FILE}" "${CMAKE_BINARY_DIR}/compile_commands.json"; then
+  ${CLANG_TIDY} -quiet -header-filter="\(${CMAKE_SOURCE_DIR}|\\.\\.\)/src/.*" --extra-arg-before=-Wno-unknown-warning-option -format-style=file -p "${CMAKE_BINARY_DIR}" "${FILE}"
+else
+  echo "Skipping ${FILE}"
+fi

src/aktualizr_get/main.cc

@@ -6,12 +6,11 @@
 
 #include "config/config.h"
 #include "get.h"
-
 #include "utilities/aktualizr_version.h"
 
 namespace bpo = boost::program_options;
 
-bpo::variables_map parse_options(int argc, char *argv[]) {
+bpo::variables_map parse_options(int argc, char **argv) {
   bpo::options_description description(
       "A tool similar to wget that will do an HTTP get on the given URL using the device's configured credentials.");
   // clang-format off

src/aktualizr_lite/main.cc

@@ -6,7 +6,6 @@
 
 #include "config/config.h"
 #include "helpers.h"
-
 #include "utilities/aktualizr_version.h"
 
 namespace bpo = boost::program_options;
@@ -157,11 +156,7 @@ struct SubCommand {
   const char *name;
   int (*main)(LiteClient &, const bpo::variables_map &);
 };
-static SubCommand commands[] = {
-    {"status", status_main},
-    {"list", list_main},
-    {"update", update_main},
-};
+static std::array<SubCommand, 3> commands{{{"status", status_main}, {"list", list_main}, {"update", update_main}}};
 
 void check_info_options(const bpo::options_description &description, const bpo::variables_map &vm) {
   if (vm.count("help") != 0 || (vm.count("command") == 0 && vm.count("version") == 0)) {
@@ -174,9 +169,9 @@ void check_info_options(const bpo::options_description &description, const bpo::
   }
 }
 
-bpo::variables_map parse_options(int argc, char *argv[]) {
+bpo::variables_map parse_options(int argc, char **argv) {
   std::string subs("Command to execute: ");
-  for (size_t i = 0; i < sizeof(commands) / sizeof(SubCommand); i++) {
+  for (size_t i = 0; i < commands.size(); i++) {
     if (i != 0) {
       subs += ", ";
     }
@@ -253,7 +248,7 @@ int main(int argc, char *argv[]) {
     LOG_DEBUG << "Current directory: " << boost::filesystem::current_path().string();
 
     std::string cmd = commandline_map["command"].as<std::string>();
-    for (size_t i = 0; i < sizeof(commands) / sizeof(SubCommand); i++) {
+    for (size_t i = 0; i < commands.size(); i++) {
       if (cmd == commands[i].name) {
         LiteClient client(config);
         return commands[i].main(client, commandline_map);

src/aktualizr_primary/main.cc

@@ -27,7 +27,7 @@ void checkInfoOptions(const bpo::options_description &description, const bpo::va
   }
 }
 
-bpo::variables_map parseOptions(int argc, char *argv[]) {
+bpo::variables_map parseOptions(int argc, char **argv) {
   bpo::options_description description("aktualizr command line options");
   // clang-format off
   // Try to keep these options in the same order as Config::updateFromCommandLine().

src/aktualizr_secondary/aktualizr_secondary.cc

@@ -118,7 +118,9 @@ data::ResultCode::Numeric AktualizrSecondary::install(const std::string& target_
       LOG_INFO << "The target has been successfully installed, but a reboot is required to be applied: " << target_name;
       break;
     }
-    default: { LOG_INFO << "Failed to install the target: " << target_name; }
+    default: {
+      LOG_INFO << "Failed to install the target: " << target_name;
+    }
   }
 
   return install_result;

src/aktualizr_secondary/main.cc

@@ -26,7 +26,7 @@ void check_secondary_options(const bpo::options_description &description, const
   }
 }
 
-bpo::variables_map parse_options(int argc, char *argv[]) {
+bpo::variables_map parse_options(int argc, char **argv) {
   bpo::options_description description("aktualizr-secondary command line options");
   // clang-format off
   description.add_options()

src/aktualizr_secondary/update_agent_ostree.cc

@@ -42,7 +42,10 @@ bool OstreeUpdateAgent::download(const Uptane::Target& target, const std::string
   bool download_result = false;
 
   try {
-    std::string ca, cert, pkey, server_url;
+    std::string ca;
+    std::string cert;
+    std::string pkey;
+    std::string server_url;
     extractCredentialsArchive(data, &ca, &cert, &pkey, &server_url);
     keyMngr_->loadKeys(&pkey, &cert, &ca);
     boost::trim(server_url);

src/cert_provider/main.cc

@@ -31,7 +31,7 @@ void checkInfoOptions(const bpo::options_description& description, const bpo::va
   }
 }
 
-bpo::variables_map parseOptions(int argc, char* argv[]) {
+bpo::variables_map parseOptions(int argc, char** argv) {
   bpo::options_description description("aktualizr-cert-provider command line options");
   // clang-format off
   description.add_options()

src/libaktualizr-c/test/api-test-utils/api-test-utils.cc

@@ -13,10 +13,12 @@ void Run_fake_http_server(const char *serverPath, const char *metaPath) {
   std::string port = TestUtils::getFreePort();
   serverAddress = "http://127.0.0.1:" + port;
 
+  // NOLINTNEXTLINE(clang-analyzer-core.NonNullParamChecker)
   server = std_::make_unique<boost::process::child>(serverPath, port, "-f", "-m", metaPath);
   TestUtils::waitForServer(serverAddress + "/");
 }
 
+// NOLINTNEXTLINE(clang-analyzer-core.CallAndMessage)
 void Stop_fake_http_server() { server.reset(); }
 
 Config *Get_test_config() {

src/libaktualizr-isotp/isotp_conn.cc

@@ -92,7 +92,7 @@ bool IsoTpSendRecv::Send(const std::string& out) {
   } else {
     while (true) {
       fd_set read_set;
-      FD_ZERO(&read_set);
+      FD_ZERO(&read_set);  // NOLINT(readability-isolate-declaration)
       FD_SET(can_socket, &read_set);
 
       // struct timeval timeout = {0, 20000};  // 20 ms
@@ -165,7 +165,7 @@ bool IsoTpSendRecv::Recv(std::string* in) {
 
   while (true) {
     fd_set read_set;
-    FD_ZERO(&read_set);
+    FD_ZERO(&read_set);  // NOLINT(readability-isolate-declaration)
     FD_SET(can_socket, &read_set);
 
     // struct timeval timeout = {0, 2000000};  // 20 ms

src/libaktualizr-isotp/isotp_conn.h

@@ -3,6 +3,7 @@
 
 #include <cstdint>
 #include <string>
+
 #include "isotp/isotp.h"
 
 class IsoTpSendRecv {

src/libaktualizr/crypto/crypto.cc

@@ -1,10 +1,11 @@
 #include "crypto.h"
 
-#include <boost/algorithm/hex.hpp>
-#include <boost/scoped_array.hpp>
+#include <array>
 #include <iostream>
 
 #include <sodium.h>
+#include <boost/algorithm/hex.hpp>
+#include <boost/scoped_array.hpp>
 
 #include "logging/logging.h"
 #include "openssl_compat.h"
@@ -94,22 +95,23 @@ Json::Value PublicKey::ToUptane() const {
 
 std::string PublicKey::KeyId() const {
   std::string key_content = value_;
+  // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks)
   boost::algorithm::trim_right_if(key_content, boost::algorithm::is_any_of("\n"));
   std::string keyid = boost::algorithm::hex(Crypto::sha256digest(Utils::jsonToCanonicalStr(Json::Value(key_content))));
   std::transform(keyid.begin(), keyid.end(), keyid.begin(), ::tolower);
   return keyid;
 }
 
 std::string Crypto::sha256digest(const std::string &text) {
-  unsigned char sha256_hash[crypto_hash_sha256_BYTES];
-  crypto_hash_sha256(sha256_hash, reinterpret_cast<const unsigned char *>(text.c_str()), text.size());
-  return std::string(reinterpret_cast<char *>(sha256_hash), crypto_hash_sha256_BYTES);
+  std::array<unsigned char, crypto_hash_sha256_BYTES> sha256_hash{};
+  crypto_hash_sha256(sha256_hash.data(), reinterpret_cast<const unsigned char *>(text.c_str()), text.size());
+  return std::string(reinterpret_cast<char *>(sha256_hash.data()), crypto_hash_sha256_BYTES);
 }
 
 std::string Crypto::sha512digest(const std::string &text) {
-  unsigned char sha512_hash[crypto_hash_sha512_BYTES];
-  crypto_hash_sha512(sha512_hash, reinterpret_cast<const unsigned char *>(text.c_str()), text.size());
-  return std::string(reinterpret_cast<char *>(sha512_hash), crypto_hash_sha512_BYTES);
+  std::array<unsigned char, crypto_hash_sha512_BYTES> sha512_hash{};
+  crypto_hash_sha512(sha512_hash.data(), reinterpret_cast<const unsigned char *>(text.c_str()), text.size());
+  return std::string(reinterpret_cast<char *>(sha512_hash.data()), crypto_hash_sha512_BYTES);
 }
 
 std::string Crypto::RSAPSSSign(ENGINE *engine, const std::string &private_key, const std::string &message) {
@@ -179,10 +181,10 @@ std::string Crypto::Sign(KeyType key_type, ENGINE *engine, const std::string &pr
 }
 
 std::string Crypto::ED25519Sign(const std::string &private_key, const std::string &message) {
-  unsigned char sig[crypto_sign_BYTES];
-  crypto_sign_detached(sig, nullptr, reinterpret_cast<const unsigned char *>(message.c_str()), message.size(),
+  std::array<unsigned char, crypto_sign_BYTES> sig{};
+  crypto_sign_detached(sig.data(), nullptr, reinterpret_cast<const unsigned char *>(message.c_str()), message.size(),
                        reinterpret_cast<const unsigned char *>(private_key.c_str()));
-  return std::string(reinterpret_cast<char *>(sig), crypto_sign_BYTES);
+  return std::string(reinterpret_cast<char *>(sig.data()), crypto_sign_BYTES);
 }
 
 bool Crypto::RSAPSSVerify(const std::string &public_key, const std::string &signature, const std::string &message) {
@@ -417,12 +419,12 @@ bool Crypto::generateRSAKeyPair(KeyType key_type, std::string *public_key, std::
 }
 
 bool Crypto::generateEDKeyPair(std::string *public_key, std::string *private_key) {
-  unsigned char pk[crypto_sign_PUBLICKEYBYTES];
-  unsigned char sk[crypto_sign_SECRETKEYBYTES];
-  crypto_sign_keypair(pk, sk);
-  *public_key = boost::algorithm::hex(std::string(reinterpret_cast<char *>(pk), crypto_sign_PUBLICKEYBYTES));
+  std::array<unsigned char, crypto_sign_PUBLICKEYBYTES> pk{};
+  std::array<unsigned char, crypto_sign_SECRETKEYBYTES> sk{};
+  crypto_sign_keypair(pk.data(), sk.data());
+  *public_key = boost::algorithm::hex(std::string(reinterpret_cast<char *>(pk.data()), crypto_sign_PUBLICKEYBYTES));
   // std::transform(public_key->begin(), public_key->end(), public_key->begin(), ::tolower);
-  *private_key = boost::algorithm::hex(std::string(reinterpret_cast<char *>(sk), crypto_sign_SECRETKEYBYTES));
+  *private_key = boost::algorithm::hex(std::string(reinterpret_cast<char *>(sk.data()), crypto_sign_SECRETKEYBYTES));
   // std::transform(private_key->begin(), private_key->end(), private_key->begin(), ::tolower);
   return true;
 }
@@ -502,7 +504,9 @@ Hash Hash::generate(Type type, const std::string &data) {
       hash = boost::algorithm::hex(Crypto::sha512digest(data));
       break;
     }
-    default: { throw std::invalid_argument("Unsupported hash type"); }
+    default: {
+      throw std::invalid_argument("Unsupported hash type");
+    }
   }
 
   return Hash(type, hash);