Merge pull request #1175 from libcpr/feature/ci-fixes

Clang-Tidy And cppcheck Fixes
libcpr · Feb 9, 2025 · c8a803c · c8a803c
2 parents 4c41606 + cfb7960
commit c8a803c
Show file tree

Hide file tree

Showing 12 changed files with 75 additions and 45 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -40,5 +40,5 @@ Checks: '*,
 -boost-use-ranges
 '
 WarningsAsErrors: '*'
-HeaderFilterRegex: 'src/*.hpp'
+HeaderFilterRegex: 'src\/*.hpp'
 FormatStyle: file
diff --git a/.github/workflows/cppcheck.yml b/.github/workflows/cppcheck.yml
@@ -17,6 +17,8 @@ jobs:
     - name: "[Release g++] Build"
       env:
         CPR_ENABLE_CPPCHECK: ON
+        # Avoid parallel runs so only the resulting error file is not being written by multiple processes at the same time.
+        CMAKE_BUILD_PARALLEL_LEVEL: 1
       uses: ashutoshvarma/action-cmake-build@master
       with:
         build-dir: ${{github.workspace}}/build

diff --git a/CppCheckSuppressions.txt b/CppCheckSuppressions.txt
diff --git a/cmake/cppcheck.cmake b/cmake/cppcheck.cmake
@@ -1,10 +1,15 @@
 find_program(CMAKE_CXX_CPPCHECK NAMES cppcheck)
+
 if(CMAKE_CXX_CPPCHECK)
-    list(APPEND CMAKE_CXX_CPPCHECK 
+    list(APPEND CMAKE_CXX_CPPCHECK "--xml"
         "--error-exitcode=1"
         "--enable=warning,style"
-        "--force" 
+        "--force"
         "--inline-suppr"
+        "--addon=y2038"
         "--std=c++${CMAKE_CXX_STANDARD}"
-        "--suppressions-list=${CMAKE_SOURCE_DIR}/CppCheckSuppressions.txt")
+        "--cppcheck-build-dir=${PROJECT_BINARY_DIR}"
+        "--suppress-xml=${PROJECT_SOURCE_DIR}/cppcheck-suppressions.xml"
+        "--output-file=${PROJECT_BINARY_DIR}/cppcheck.xml"
+        "--check-level=normal")
 endif()
diff --git a/cppcheck-suppressions.xml b/cppcheck-suppressions.xml
@@ -0,0 +1,37 @@
+<?xml version="1.0"?>
+<suppressions>
+    <!-- Exclude Paths -->
+    <suppress>
+        <id>*</id>
+        <fileName>*/build/*</fileName>
+    </suppress>
+    <suppress>
+        <id>CheckLevelMaxBranches</id>
+    </suppress>
+    <suppress>
+        <id>noExplicitConstructor</id>
+    </suppress>
+    <suppress>
+        <id>knownConditionTrueFalse</id>
+        <fileName>*/include/cpr/async_wrapper.h</fileName>
+    </suppress>
+    <suppress>
+        <id>y2038-unsafe-call</id>
+        <fileName>*/cpr/cookies.cpp</fileName>
+    </suppress>
+    <!-- Known Limitation/Bug: https://github.com/libcpr/cpr/issues/1174 -->
+    <suppress>
+        <id>y2038-unsafe-call</id>
+        <fileName>*/include/cpr/low_speed.h</fileName>
+    </suppress>
+    <suppress>
+        <id>normalCheckLevelMaxBranches</id>
+    </suppress>
+    <suppress>
+        <id>constParameterPointer</id>
+        <fileName>*/cpr/util.cpp</fileName>
+    </suppress>
+    <suppress>
+        <id>postfixOperator</id>
+    </suppress>
+</suppressions>
diff --git a/cpr/cookies.cpp b/cpr/cookies.cpp
@@ -8,27 +8,27 @@
 #include <string_view>
 
 namespace cpr {
-const std::string Cookie::GetDomain() const {
+const std::string& Cookie::GetDomain() const {
     return domain_;
 }
 
 bool Cookie::IsIncludingSubdomains() const {
     return includeSubdomains_;
 }
 
-const std::string Cookie::GetPath() const {
+const std::string& Cookie::GetPath() const {
     return path_;
 }
 
 bool Cookie::IsHttpsOnly() const {
     return httpsOnly_;
 }
 
-const std::chrono::system_clock::time_point Cookie::GetExpires() const {
+std::chrono::system_clock::time_point Cookie::GetExpires() const {
     return expires_;
 }
 
-const std::string Cookie::GetExpiresString() const {
+std::string Cookie::GetExpiresString() const {
     std::stringstream ss;
     std::tm tm{};
     const std::time_t tt = std::chrono::system_clock::to_time_t(expires_);
@@ -42,15 +42,15 @@ const std::string Cookie::GetExpiresString() const {
     return ss.str();
 }
 
-const std::string Cookie::GetName() const {
+const std::string& Cookie::GetName() const {
     return name_;
 }
 
-const std::string Cookie::GetValue() const {
+const std::string& Cookie::GetValue() const {
     return value_;
 }
 
-const std::string Cookies::GetEncoded(const CurlHolder& holder) const {
+std::string Cookies::GetEncoded(const CurlHolder& holder) const {
     std::stringstream stream;
     for (const cpr::Cookie& item : cookies_) {
         // Depending on if encoding is set to "true", we will URL-encode cookies

diff --git a/cpr/proxyauth.cpp b/cpr/proxyauth.cpp
@@ -1,6 +1,6 @@
 #include "cpr/proxyauth.h"
-#include "cpr/util.h"
 #include <string>
+#include <string_view>
 
 namespace cpr {
 

diff --git a/cpr/session.cpp b/cpr/session.cpp
@@ -184,8 +184,8 @@ void Session::prepareCommonShared() {
     // handle NO_PROXY override passed through Proxies object
     // Example: Proxies{"no_proxy": ""} will override environment variable definition with an empty list
     const std::array<std::string, 2> no_proxy{"no_proxy", "NO_PROXY"};
-    for (const auto& item : no_proxy) {
-        if (proxies_.has(item)) {
+    for (const auto& item : no_proxy) { // cppcheck-suppress useStlAlgorithm
+        if (proxies_.has(item)) {       // cppcheck-suppress useStlAlgorithm
             curl_easy_setopt(curl_->handle, CURLOPT_NOPROXY, proxies_[item].c_str());
             break;
         }
@@ -449,7 +449,7 @@ void Session::SetBody(Body&& body) {
 
 void Session::SetLowSpeed(const LowSpeed& low_speed) {
     curl_easy_setopt(curl_->handle, CURLOPT_LOW_SPEED_LIMIT, low_speed.limit);
-    curl_easy_setopt(curl_->handle, CURLOPT_LOW_SPEED_TIME, low_speed.time);
+    curl_easy_setopt(curl_->handle, CURLOPT_LOW_SPEED_TIME, low_speed.time); // cppcheck-suppress y2038-unsafe-call
 }
 
 void Session::SetVerifySsl(const VerifySsl& verify) {

diff --git a/cpr/threadpool.cpp b/cpr/threadpool.cpp
@@ -111,9 +111,7 @@ bool ThreadPool::CreateThread() {
             if (task) {
                 task();
                 ++idle_thread_num;
-                if (initialRun) {
-                    initialRun = false;
-                }
+                initialRun = false;
             }
         }
     });

diff --git a/cpr/util.cpp b/cpr/util.cpp
@@ -3,6 +3,7 @@
 #include "cpr/cookies.h"
 #include "cpr/cprtypes.h"
 #include "cpr/curlholder.h"
+#include "cpr/secure_string.h"
 #include <algorithm>
 #include <cctype>
 #include <chrono>
@@ -11,7 +12,6 @@
 #include <curl/curl.h>
 #include <fstream>
 #include <ios>
-#include <iterator>
 #include <sstream>
 #include <string>
 #include <type_traits>
@@ -189,8 +189,7 @@ util::SecureString urlDecode(std::string_view s) {
 
 bool isTrue(const std::string& s) {
     constexpr std::string_view tmp = "true";
-    auto [s_it, tmp_it] = std::mismatch(s.begin(), s.end(), tmp.begin(), tmp.end(),
-        [](auto s_c, auto t_c) { return std::tolower(s_c) == t_c; });
+    auto [s_it, tmp_it] = std::mismatch(s.begin(), s.end(), tmp.begin(), tmp.end(), [](auto s_c, auto t_c) { return std::tolower(s_c) == t_c; });
     return s_it == s.end() && tmp_it == tmp.end();
 }
 

diff --git a/include/cpr/cookies.h b/include/cpr/cookies.h
@@ -4,7 +4,6 @@
 #include "cpr/curlholder.h"
 #include <chrono>
 #include <initializer_list>
-#include <sstream>
 #include <string>
 #include <vector>
 
@@ -24,14 +23,14 @@ class Cookie {
      * So we fall back to std::chrono::system_clock::from_time_t(0) for the minimum value here.
      **/
     Cookie(const std::string& name, const std::string& value, const std::string& domain = "", bool p_isIncludingSubdomains = false, const std::string& path = "/", bool p_isHttpsOnly = false, std::chrono::system_clock::time_point expires = std::chrono::system_clock::from_time_t(0)) : name_{name}, value_{value}, domain_{domain}, includeSubdomains_{p_isIncludingSubdomains}, path_{path}, httpsOnly_{p_isHttpsOnly}, expires_{expires} {}
-    const std::string GetDomain() const;
-    bool IsIncludingSubdomains() const;
-    const std::string GetPath() const;
-    bool IsHttpsOnly() const;
-    const std::chrono::system_clock::time_point GetExpires() const;
-    const std::string GetExpiresString() const;
-    const std::string GetName() const;
-    const std::string GetValue() const;
+    [[nodiscard]] const std::string& GetDomain() const;
+    [[nodiscard]] bool IsIncludingSubdomains() const;
+    [[nodiscard]] const std::string& GetPath() const;
+    [[nodiscard]] bool IsHttpsOnly() const;
+    [[nodiscard]] std::chrono::system_clock::time_point GetExpires() const;
+    [[nodiscard]] std::string GetExpiresString() const;
+    [[nodiscard]] const std::string& GetName() const;
+    [[nodiscard]] const std::string& GetValue() const;
 
   private:
     std::string name_;
@@ -68,17 +67,17 @@ class Cookies {
     Cookies(const cpr::Cookie& cookie, bool p_encode = true) : encode{p_encode}, cookies_{cookie} {}
 
     cpr::Cookie& operator[](size_t pos);
-    const std::string GetEncoded(const CurlHolder& holder) const;
+    [[nodiscard]] std::string GetEncoded(const CurlHolder& holder) const;
 
     using iterator = std::vector<cpr::Cookie>::iterator;
     using const_iterator = std::vector<cpr::Cookie>::const_iterator;
 
     iterator begin();
     iterator end();
-    const_iterator begin() const;
-    const_iterator end() const;
-    const_iterator cbegin() const;
-    const_iterator cend() const;
+    [[nodiscard]] const_iterator begin() const;
+    [[nodiscard]] const_iterator end() const;
+    [[nodiscard]] const_iterator cbegin() const;
+    [[nodiscard]] const_iterator cend() const;
     void emplace_back(const Cookie& str);
     [[nodiscard]] bool empty() const;
     void push_back(const Cookie& str);

diff --git a/test/get_tests.cpp b/test/get_tests.cpp
@@ -15,13 +15,6 @@ using namespace cpr;
 
 static HttpServer* server = new HttpServer();
 
-TEST(BasicTests, XXXTest) {
-    Url url{"https://getsolara.dev/api/endpoint.json"};
-    Response response = cpr::Get(url);
-    EXPECT_EQ(200, response.status_code);
-    EXPECT_EQ(ErrorCode::OK, response.error.code);
-}
-
 TEST(BasicTests, HelloWorldTest) {
     Url url{server->GetBaseUrl() + "/hello.html"};
     Response response = cpr::Get(url);