Fixed some compiler warnings and issues found by static code alalysis…

…. Also fixed a few issues with the windows version.
jgaa · Aug 17, 2024 · c688196 · c688196
1 parent cb2b65f
commit c688196
Show file tree

Hide file tree

Showing 37 changed files with 714 additions and 716 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -1,11 +1,5 @@
 cmake_minimum_required(VERSION 3.10)
 
-# if (UNIX)
-#     # Ninja creates too many problems while maintaining
-#     # compatibility with old and new versions of Linux/Cmake
-#     set (CMAKE_GENERATOR "Unix Makefiles" CACHE INTERNAL "" FORCE)
-# endif()
-
 if (DEFINED ENV{RESTC_CPP_VERSION})
     set(RESTC_CPP_VERSION $ENV{RESTC_CPP_VERSION})
 endif()
@@ -28,6 +22,7 @@ if (MSVC)
     # Thank you Microsoft. Its so nice of you to give us all these meaningful reasons to stay up all night.
     check_cxx_compiler_flag("/std:c++20" COMPILER_SUPPORTS_CXX20)
     check_cxx_compiler_flag("/std:c++17" COMPILER_SUPPORTS_CXX17)
+    add_compile_options(/Zc:__cplusplus)
 else()
     check_cxx_compiler_flag("-std=c++20" COMPILER_SUPPORTS_CXX20)
     check_cxx_compiler_flag("-std=c++17" COMPILER_SUPPORTS_CXX17)
@@ -303,10 +298,6 @@ if (NOT EMBEDDED_RESTC_CPP)
         link_directories(${Boost_LIBRARY_DIRS})
     endif()
 
-    include(cmake_scripts/pch.cmake)
-
-    set_property(TARGET  PROPERTY CXX_STANDARD 17)
-
     if(WIN32)
         add_definitions(-D_WIN32_WINNT=0x0600)
         set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /SAFESEH:NO")

diff --git a/examples/logip/logip.cpp b/examples/logip/logip.cpp
@@ -39,12 +39,13 @@ BOOST_FUSION_ADAPT_STRUCT(
 
 string now() {
     char date[32] = {};
-    auto now = time(NULL);
+    auto now = time(nullptr);
     strftime(date, sizeof(date), "%Y-%m-%d %H:%M", localtime(&now));
     return date;
 }
 
-int main(int argc, char *argv[]) {
+int main(int /*argc*/, char * /*argv*/[])
+{
 #ifdef RESTC_CPP_LOG_WITH_BOOST_LOG
     namespace logging = boost::log;
     logging::core::get()->set_filter
@@ -71,22 +72,14 @@ int main(int argc, char *argv[]) {
                     .Execute());
                 valid = true;
             } catch (const boost::exception& ex) {
-                clog << now()
-                    << "Caught boost exception: "
-                    << boost::diagnostic_information(ex)
-                    << endl;
+                clog << now() << "Caught boost exception: " << boost::diagnostic_information(ex)
+                     << '\n';
             } catch (const exception& ex) {
-                clog << now()
-                    << "Caught exception: "
-                    << ex.what()
-                    << endl;
+                clog << now() << "Caught exception: " << ex.what() << '\n';
             }
 
             if (valid && (current_ip != data.ip)) {
-                clog << now()
-                    << ' '
-                    << data.ip
-                    << endl;
+                clog << now() << ' ' << data.ip << '\n';
                 current_ip = data.ip;
             }
 

diff --git a/include/restc-cpp/IteratorFromJsonSerializer.h b/include/restc-cpp/IteratorFromJsonSerializer.h
@@ -18,12 +18,13 @@ class IteratorFromJsonSerializer
 public:
     using data_t = typename std::remove_const<typename std::remove_reference<objectT>::type>::type;
 
-    class Iterator : public std::iterator<
-        std::input_iterator_tag,
-        data_t,
-        std::ptrdiff_t,
-        const data_t *,
-        data_t&> {
+    class Iterator {
+    public:
+        using iterator_category = std::input_iterator_tag;
+        using value_type = data_t;
+        using difference_type = std::ptrdiff_t;
+        using pointer = value_type*;
+        using reference = value_type&;
 
     public:
         Iterator() {}
@@ -42,7 +43,7 @@ class IteratorFromJsonSerializer
         }
 
         Iterator(Iterator&& it)
-        : owner_{it.owner_}, data_{move(it.data_)} {}
+            : owner_{it.owner_}, data_{std::move(it.data_)} {}
 
         Iterator(IteratorFromJsonSerializer *owner)
         : owner_{owner} {}
@@ -70,7 +71,7 @@ class IteratorFromJsonSerializer
 
         Iterator& operator = (Iterator&& it) {
             owner_ = it.owner_;
-            it.data_ = move(it.data_);
+            it.data_ = std::move(it.data_);
         }
 
         bool operator == (const Iterator& other) const {
@@ -170,7 +171,7 @@ class IteratorFromJsonSerializer
                     RapidJsonDeserializer<objectT> handler(
                         *data, *properties_);
                     json_reader_.Parse(reply_stream_, handler);
-                    return move(data);
+                    return std::move(data);
                 } else if (ch == ']') {
                     reply_stream_.Take();
                     state_ = State::DONE;

diff --git a/include/restc-cpp/RequestBuilder.h b/include/restc-cpp/RequestBuilder.h
@@ -187,7 +187,7 @@ class RequestBuilder
             args_ = Request::args_t();
         }
 
-        args_->push_back({move(name), move(value)});
+        args_->push_back({std::move(name), std::move(value)});
         return *this;
     }
 
@@ -202,14 +202,14 @@ class RequestBuilder
      * \param value Value of the argument
      */
     RequestBuilder& Argument(std::string name, int64_t value) {
-        return Argument(move(name), std::to_string(value));
+        return Argument(std::move(name), std::to_string(value));
     } 
 
     /*! Supply your own RequestBody to the request
      */
     RequestBuilder& Body(std::unique_ptr<RequestBody> body) {
         assert(!body_);
-        body_ = move(body);
+        body_ = std::move(body);
         return *this;
     }
 
@@ -252,7 +252,7 @@ class RequestBuilder
      */
     RequestBuilder& Data(std::string&& body) {
         assert(!body_);
-        body_ = RequestBody::CreateStringBody(move(body));
+        body_ = RequestBody::CreateStringBody(std::move(body));
         return *this;
     }
 
@@ -368,7 +368,7 @@ class RequestBuilder
         }
 #endif
         auto req = Request::Create(
-            url_, type_, ctx_->GetClient(), move(body_), args_, headers_, auth_);
+            url_, type_, ctx_->GetClient(), std::move(body_), args_, headers_, auth_);
 
         auto orig = req->GetProperties();
 

diff --git a/include/restc-cpp/logging.h b/include/restc-cpp/logging.h
@@ -22,6 +22,7 @@
 #include <thread>
 #include <iomanip>
 #include <array>
+#include <ctime>
 
 namespace restc_cpp  {
 
@@ -108,6 +109,17 @@ class Logger {
 
 }
 
+
+inline std::tm *restc_cpp_localtime(const time_t now, std::tm& timeInfo) {
+#ifdef _WIN32
+    localtime_s(&timeInfo, &now); // Windows-specific
+#else
+    localtime_r(&now, &timeInfo); // POSIX-specific
+#endif
+
+    return &timeInfo;
+}
+
 //#define RESTC_CPP_TEST_LOGGING_SETUP(level) RestcCppTestStartLogger(level)
 #define RESTC_CPP_TEST_LOGGING_SETUP(level) RestcCppTestStartLogger("trace")
 
@@ -130,9 +142,9 @@ inline void RestcCppTestStartLogger(const std::string& level = "info") {
                                              const std::string& msg) {
         static const std::array<std::string, 6> levels = {"NONE", "ERROR", "WARN", "INFO", "DEBUG", "TRACE"};
 
-        const auto now = std::chrono::system_clock::to_time_t(std::chrono::system_clock::now());
+        std::tm timeInfo = {};
 
-        std::clog << std::put_time(std::localtime(&now), "%c") << ' '
+        std::clog << std::put_time(restc_cpp_localtime(time({}), timeInfo), "%c") << ' '
                   << levels.at(static_cast<size_t>(level))
                   << ' ' << std::this_thread::get_id() << ' '
                   << msg << std::endl;

diff --git a/include/restc-cpp/restc-cpp.h b/include/restc-cpp/restc-cpp.h
@@ -140,7 +140,7 @@ class Request {
         Type type = Type::NONE;
         std::string address;
 
-        const std::string& GetName();
+        const std::string &GetName() const;
     };
 
     using args_t = std::deque<Arg>;
@@ -423,7 +423,7 @@ class RestClient {
             done_handler.reset();
         });
 
-        return move(future);
+        return future;
     }
 
     /*! Process from within an existing coroutine */

diff --git a/include/restc-cpp/test_helper.h b/include/restc-cpp/test_helper.h
@@ -14,10 +14,27 @@ namespace restc_cpp {
 // Substitute localhost with whatever is in the environment-variable
 // RESTC_CPP_TEST_DOCKER_ADDRESS
 inline std::string GetDockerUrl(std::string url) {
-    const char *docker_addr = std::getenv("RESTC_CPP_TEST_DOCKER_ADDRESS");
+#ifdef _WIN32
+    // On Windows, use _dupenv_s to safely retrieve the environment variable
+    size_t len = 0;
+    char* docker_addr = nullptr;
+    errno_t err = _dupenv_s(&docker_addr, &len, "RESTC_CPP_TEST_DOCKER_ADDRESS");
+    if (err != 0 || docker_addr == nullptr) {
+        docker_addr = nullptr;  // Ensure docker_addr is nullptr if the variable isn't set
+    }
+#else
+    // On Linux/macOS, use std::getenv
+    auto docker_addr = std::getenv("RESTC_CPP_TEST_DOCKER_ADDRESS");
+#endif
+
     if (docker_addr) {
         boost::replace_all(url, "localhost", docker_addr);
+#ifdef _WIN32
+        // Free the allocated memory on Windows
+        free(docker_addr);
+#endif
     }
+
     return url;
 }
 

diff --git a/src/ChunkedReaderImpl.cpp b/src/ChunkedReaderImpl.cpp
@@ -17,13 +17,11 @@ class ChunkedReaderImpl : public DataReader {
 public:
 
     ChunkedReaderImpl(add_header_fn_t&& fn, unique_ptr<DataReaderStream>&& source)
-    : stream_{move(source)}, add_header_(move(fn))
+    : stream_{std::move(source)}, add_header_(std::move(fn))
     {
     }
 
-    bool IsEof() const override {
-        return stream_->IsEof();
-    }
+    [[nodiscard]] bool IsEof() const override { return stream_->IsEof(); }
 
     void Finish() override {
         ReadSome();
@@ -36,15 +34,16 @@ class ChunkedReaderImpl : public DataReader {
         }
     }
 
-    string ToPrintable(boost::string_ref buf) const {
+    [[nodiscard]] static string ToPrintable(boost::string_ref buf)
+    {
         ostringstream out;
-        locale loc;
+        locale const loc;
         auto pos = 0;
-        out << endl;
+        out << '\n';
 
         for(const auto ch : buf) {
-            if (!(++pos % line_length)) {
-                out << endl;
+            if ((++pos % line_length) == 0u) {
+                out << '\n';
             }
             if (std::isprint(ch, loc)) {
                 out << ch;
@@ -56,7 +55,8 @@ class ChunkedReaderImpl : public DataReader {
         return out.str();
     }
 
-    void Log(const boost::asio::const_buffers_1 buffers, const char *tag) {
+    static void Log(const boost::asio::const_buffers_1 buffers, const char * /*tag*/)
+    {
         const auto buf_len = boost::asio::buffer_size(*buffers.begin());
 
         // At the time of the implementation, there are never multiple buffers.
@@ -103,12 +103,11 @@ class ChunkedReaderImpl : public DataReader {
         if (eat_chunk_padding_) {
             eat_chunk_padding_ = false;
 
-            char ch = {};
-            if ((ch = stream_->Getc()) != '\r') {
+            if (stream_->Getc() != '\r') {
                 throw ParseException("Chunk: Missing padding CR!");
             }
 
-            if ((ch = stream_->Getc()) != '\n') {
+            if (stream_->Getc() != '\n') {
                 throw ParseException("Chunk: Missing padding LF!");
             }
         }
@@ -133,11 +132,11 @@ class ChunkedReaderImpl : public DataReader {
         size_t chunk_len = 0;
         char ch = stream_->Getc();
 
-        if (!isxdigit(ch)) {
+        if (isxdigit(ch) == 0) {
             throw ParseException("Missing chunk-length in new chunk.");
         }
 
-        for(; isxdigit(ch); ch = stream_->Getc()) {
+        for (; isxdigit(ch) != 0; ch = stream_->Getc()) {
             chunk_len *= magic_16;
             if (ch >= 'a') {
                 chunk_len += magic_10 + (ch - 'a');
@@ -148,8 +147,9 @@ class ChunkedReaderImpl : public DataReader {
             }
         }
 
-        for(; ch != '\r'; ch = stream_->Getc())
+        for (; ch != '\r'; ch = stream_->Getc()) {
             ;
+        }
 
         if (ch != '\r') {
             throw ParseException("Missing CR in first chunk line");
@@ -171,7 +171,7 @@ class ChunkedReaderImpl : public DataReader {
 
 DataReader::ptr_t
 DataReader::CreateChunkedReader(add_header_fn_t fn, unique_ptr<DataReaderStream>&& source) {
-    return make_unique<ChunkedReaderImpl>(move(fn), move(source));
+    return make_unique<ChunkedReaderImpl>(std::move(fn), std::move(source));
 }
 
 

diff --git a/src/ChunkedWriterImpl.cpp b/src/ChunkedWriterImpl.cpp
@@ -16,7 +16,7 @@ namespace restc_cpp {
 class ChunkedWriterImpl : public DataWriter {
 public:
     ChunkedWriterImpl(add_header_fn_t fn, ptr_t&& source)
-    : next_{move(source)},  add_header_fn_{move(fn)}
+        : next_{std::move(source)},  add_header_fn_{std::move(fn)}
     {
     }
 
@@ -34,9 +34,7 @@ class ChunkedWriterImpl : public DataWriter {
     void Write(const write_buffers_t& buffers) override {
         const auto len = boost::asio::buffer_size(buffers);
         buffers_.resize(1);
-        for(auto &b : buffers) {
-            buffers_.push_back(b);
-        }
+        std::copy(buffers.begin(), buffers.end(), std::back_inserter(buffers_));
         DoWrite(len);
     }
 
@@ -101,7 +99,7 @@ class ChunkedWriterImpl : public DataWriter {
 
 DataWriter::ptr_t
 DataWriter::CreateChunkedWriter(add_header_fn_t fn, ptr_t&& source) {
-    return make_unique<ChunkedWriterImpl>(move(fn), move(source));
+    return make_unique<ChunkedWriterImpl>(std::move(fn), std::move(source));
 }
 
 } // namespace