From d743b67f99cee87520020f53b4a585f14b76bbe9 Mon Sep 17 00:00:00 2001 From: jbaldwin Date: Thu, 3 Oct 2024 13:51:52 -0600 Subject: [PATCH] async_promise_type possibly unitialized value The std::variant for callback or future/promise with gcc-13 seems to be triggering an uninitialized value warning. We can tell it isn't uninitialized but the compiler seems to be thinking the 2nd type in the variant, which is the callback type, is unitialized _when_ the promise type, 3rd type in the variant, is the one being initialized. Closes #153 --- .githooks/pre-commit | 2 +- CMakeLists.txt | 1 + README.md | 8 ++++---- examples/benchmark.cpp | 13 +++++++------ examples/readme.cpp | 8 ++++---- inc/lift/executor.hpp | 6 +++--- inc/lift/http.hpp | 2 +- inc/lift/impl/pragma.hpp | 19 +++++++++++++++++++ inc/lift/lift_status.hpp | 2 +- inc/lift/query_builder.hpp | 2 +- inc/lift/request.hpp | 12 +++++++++--- inc/lift/resolve_host.hpp | 8 ++++---- inc/lift/response.hpp | 6 +++--- inc/lift/share.hpp | 6 +++--- src/executor.cpp | 14 ++++++++++---- 15 files changed, 71 insertions(+), 38 deletions(-) create mode 100644 inc/lift/impl/pragma.hpp diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 3cabcfb..fbac14b 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -12,7 +12,7 @@ should_clang_format() { local result=0 # Ignore the test/catch_amalgamated.hpp|cpp files - if [[ "$1" != *"catch_amalgamated"* ]]; then + if [[ "$1" != *"catch_amalgamated"* && "$1" != *"pragma"* ]]; then for ext in $FILE_EXTS; do # Otherwise, if the extension is in the array of extensions to reformat, echo 1. [[ "$ext" == "$extension" ]] && result=1 && break diff --git a/CMakeLists.txt b/CMakeLists.txt index ed08bb8..a64900f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,6 +30,7 @@ message("${PROJECT_NAME} LIFT_USER_LINK_LIBRARIES = ${LIFT_USER_LINK_LIBRARIES}" set(LIBLIFTHTTP_SOURCE_FILES inc/lift/impl/copy_util.hpp + inc/lift/impl/pragma.hpp inc/lift/client.hpp src/client.cpp inc/lift/const.hpp diff --git a/README.md b/README.md index 42ef298..1f426dd 100644 --- a/README.md +++ b/README.md @@ -47,9 +47,8 @@ int main() // Debug information about any request can be added by including a callback handler for debug // information. Just pass in a lambda to capture the verbose debug information. sync_request.debug_info_handler( - [](const lift::request& /*unused*/, lift::debug_info_type type, std::string_view data) { - std::cout << "sync_request (" << lift::to_string(type) << "): " << data; - }); + [](const lift::request& /*unused*/, lift::debug_info_type type, std::string_view data) + { std::cout << "sync_request (" << lift::to_string(type) << "): " << data; }); // Set the http method for this synchronous request. sync_request.method(lift::http::method::post); @@ -96,7 +95,8 @@ int main() // Start the request that will be completed by callback. client.start_request( std::move(async_callback_request), - [](lift::request_ptr async_callback_request_returned, lift::response async_callback_response) { + [](lift::request_ptr async_callback_request_returned, lift::response async_callback_response) + { // This on complete callback will run on the lift::client background event loop thread. std::cout << "Lift status (async callback): "; std::cout << lift::to_string(async_callback_response.lift_status()) << "\n"; diff --git a/examples/benchmark.cpp b/examples/benchmark.cpp index cad66b6..0a0d4d3 100644 --- a/examples/benchmark.cpp +++ b/examples/benchmark.cpp @@ -38,11 +38,11 @@ int main(int argc, char* argv[]) { constexpr char short_options[] = "c:d:t:h"; constexpr option long_options[] = { - {"help", no_argument, nullptr, 'h'}, - {"connections", required_argument, nullptr, 'c'}, - {"duration", required_argument, nullptr, 'd'}, - {"threads", required_argument, nullptr, 't'}, - {nullptr, 0, nullptr, 0}}; + {"help", no_argument, nullptr, 'h'}, + {"connections", required_argument, nullptr, 'c'}, + {"duration", required_argument, nullptr, 'd'}, + {"threads", required_argument, nullptr, 't'}, + {nullptr, 0, nullptr, 0}}; int option_index = 0; int opt = 0; @@ -109,7 +109,8 @@ int main(int argc, char* argv[]) clients.emplace_back(std::make_unique()); callbacks.emplace_back( - [&clients, &success, &error, &callbacks, i](lift::request_ptr req_ptr, lift::response response) { + [&clients, &success, &error, &callbacks, i](lift::request_ptr req_ptr, lift::response response) + { if (response.lift_status() == lift::lift_status::success) { success.fetch_add(1, std::memory_order_relaxed); diff --git a/examples/readme.cpp b/examples/readme.cpp index 2a6e18e..a6d01d5 100644 --- a/examples/readme.cpp +++ b/examples/readme.cpp @@ -14,9 +14,8 @@ int main() // Debug information about any request can be added by including a callback handler for debug // information. Just pass in a lambda to capture the verbose debug information. sync_request.debug_info_handler( - [](const lift::request& /*unused*/, lift::debug_info_type type, std::string_view data) { - std::cout << "sync_request (" << lift::to_string(type) << "): " << data; - }); + [](const lift::request& /*unused*/, lift::debug_info_type type, std::string_view data) + { std::cout << "sync_request (" << lift::to_string(type) << "): " << data; }); // Set the http method for this synchronous request. sync_request.method(lift::http::method::post); @@ -63,7 +62,8 @@ int main() // Start the request that will be completed by callback. client.start_request( std::move(async_callback_request), - [](lift::request_ptr async_callback_request_returned, lift::response async_callback_response) { + [](lift::request_ptr async_callback_request_returned, lift::response async_callback_response) + { // This on complete callback will run on the lift::client background event loop thread. std::cout << "Lift status (async callback): "; std::cout << lift::to_string(async_callback_response.lift_status()) << "\n"; diff --git a/inc/lift/executor.hpp b/inc/lift/executor.hpp index f51bb41..4b9d949 100644 --- a/inc/lift/executor.hpp +++ b/inc/lift/executor.hpp @@ -34,10 +34,10 @@ class executor friend request; public: - executor(const executor&) = delete; - executor(executor&&) = delete; + executor(const executor&) = delete; + executor(executor&&) = delete; auto operator=(const executor&) -> executor& = delete; - auto operator=(executor&&) -> executor& = delete; + auto operator=(executor&&) -> executor& = delete; ~executor(); private: diff --git a/inc/lift/http.hpp b/inc/lift/http.hpp index 8f2c6c4..d206291 100644 --- a/inc/lift/http.hpp +++ b/inc/lift/http.hpp @@ -1,7 +1,7 @@ #pragma once -#include #include +#include #include diff --git a/inc/lift/impl/pragma.hpp b/inc/lift/impl/pragma.hpp new file mode 100644 index 0000000..40eafba --- /dev/null +++ b/inc/lift/impl/pragma.hpp @@ -0,0 +1,19 @@ +#if defined(__GNUC__) && !defined(__llvm__) + #define DO_PRAGMA(X) _Pragma(#X) + #define DISABLE_WARNING_PUSH DO_PRAGMA(GCC diagnostic push) + #define DISABLE_WARNING_POP DO_PRAGMA(GCC diagnostic pop) + #define DISABLE_WARNING(warningName) DO_PRAGMA(GCC diagnostic ignored #warningName) + + #define DISABLE_WARNING_MAYBE_UNINITIALIZED DISABLE_WARNING(-Wmaybe-uninitialized) +#elif defined(__clang__) + #define DO_PRAGMA(X) _Pragma(#X) + #define DISABLE_WARNING_PUSH + #define DISABLE_WARNING_POP + #define DISABLE_WARNING(warningNumber) + + #define DISABLE_WARNING_MAYBE_UNINITIALIZED +#else + // unknown compiler, ignoring suppression directives + #define DISABLE_WARNING_PUSH + #define DISABLE_WARNING_POP +#endif diff --git a/inc/lift/lift_status.hpp b/inc/lift/lift_status.hpp index 0fa8de7..2e68fae 100644 --- a/inc/lift/lift_status.hpp +++ b/inc/lift/lift_status.hpp @@ -1,7 +1,7 @@ #pragma once -#include #include +#include namespace lift { diff --git a/inc/lift/query_builder.hpp b/inc/lift/query_builder.hpp index 152844e..2f7af29 100644 --- a/inc/lift/query_builder.hpp +++ b/inc/lift/query_builder.hpp @@ -1,10 +1,10 @@ #pragma once +#include #include #include #include #include -#include namespace lift { diff --git a/inc/lift/request.hpp b/inc/lift/request.hpp index f6163f2..f419a7e 100644 --- a/inc/lift/request.hpp +++ b/inc/lift/request.hpp @@ -3,6 +3,7 @@ #include "lift/header.hpp" #include "lift/http.hpp" #include "lift/impl/copy_util.hpp" +#include "lift/impl/pragma.hpp" #include "lift/mime_field.hpp" #include "lift/resolve_host.hpp" #include "lift/response.hpp" @@ -155,10 +156,10 @@ class request return std::make_unique(std::move(url), std::move(timeout)); } - request(const request&) = default; - request(request&&) = default; + request(const request&) = default; + request(request&&) = default; auto operator=(const request&) noexcept -> request& = default; - auto operator=(request&&) noexcept -> request& = default; + auto operator=(request&&) noexcept -> request& = default; /** * Synchronously executes this request. @@ -538,7 +539,12 @@ class request */ auto async_future() -> async_future_type { + // gcc-13 is incorrectly thinking the 2nd type in the async_handlers_type is uninitialized. + // Its correct since it isn't used on this code path, but it isn't a warning/error. + DISABLE_WARNING_PUSH + DISABLE_WARNING_MAYBE_UNINITIALIZED m_on_complete_handler.m_object = {async_promise_type{}}; + DISABLE_WARNING_POP return std::get(m_on_complete_handler.m_object.value()).get_future(); } diff --git a/inc/lift/resolve_host.hpp b/inc/lift/resolve_host.hpp index 29ddd92..ba0cf49 100644 --- a/inc/lift/resolve_host.hpp +++ b/inc/lift/resolve_host.hpp @@ -1,7 +1,7 @@ #pragma once -#include #include +#include namespace lift { @@ -22,9 +22,9 @@ class resolve_host resolve_host(std::string resolve_host, uint16_t resolve_port, std::string resolved_ip_addr); ~resolve_host() = default; - resolve_host(const resolve_host&) = default; - resolve_host(resolve_host&&) noexcept = default; - auto operator=(const resolve_host&) -> resolve_host& = default; + resolve_host(const resolve_host&) = default; + resolve_host(resolve_host&&) noexcept = default; + auto operator=(const resolve_host&) -> resolve_host& = default; auto operator=(resolve_host&&) noexcept -> resolve_host& = default; /** diff --git a/inc/lift/response.hpp b/inc/lift/response.hpp index 61188bc..738dbea 100644 --- a/inc/lift/response.hpp +++ b/inc/lift/response.hpp @@ -27,10 +27,10 @@ class response response(); ~response() = default; - response(const response&) = default; - response(response&&) = default; + response(const response&) = default; + response(response&&) = default; auto operator=(const response&) noexcept -> response& = default; - auto operator=(response&&) noexcept -> response& = default; + auto operator=(response&&) noexcept -> response& = default; /** * The Lift status is how the request ended up in the event loop. diff --git a/inc/lift/share.hpp b/inc/lift/share.hpp index 13f3dde..c37f8ee 100644 --- a/inc/lift/share.hpp +++ b/inc/lift/share.hpp @@ -42,10 +42,10 @@ class share : public std::enable_shared_from_this explicit share(options opts); ~share(); - share(const share&) = delete; - share(share&&) = delete; + share(const share&) = delete; + share(share&&) = delete; auto operator=(const share&) noexcept -> share& = delete; - auto operator=(share&&) noexcept -> share& = delete; + auto operator=(share&&) noexcept -> share& = delete; static auto make_shared(options opts) -> std::shared_ptr { return std::make_shared(std::move(opts)); } diff --git a/src/executor.cpp b/src/executor.cpp index e162cba..c46ef1c 100644 --- a/src/executor.cpp +++ b/src/executor.cpp @@ -73,6 +73,8 @@ auto executor::prepare() -> void switch (m_request->method()) { + default: + /* INTENTIONAL FALLTHROUGH */ case http::method::unknown: // default to GET on unknown/bad value. /* INTENTIONAL FALLTHROUGH */ case http::method::get: @@ -103,6 +105,8 @@ auto executor::prepare() -> void switch (m_request->version()) { + default: + /* INTENTIONAL FALLTHROUGH */ case http::version::unknown: // default to USE_BEST on unknown/bad value. /* INTENTIONAL FALLTHROUGH */ case http::version::use_best: @@ -223,6 +227,9 @@ auto executor::prepare() -> void { switch (auth_type) { + default: + // CURLAUTH_BASIC is the default per docs https://curl.se/libcurl/c/CURLOPT_PROXYAUTH.html. + /* INTENTIONAL FALLTHROUGH */ case http_auth_type::basic: auth_types |= CURLAUTH_BASIC; break; @@ -536,10 +543,9 @@ auto curl_write_data(void* buffer, size_t size, size_t nitems, void* user_ptr) - auto& response = executor_ptr->m_response; size_t data_length = size * nitems; - std::copy( - static_cast(buffer), - static_cast(buffer) + data_length, - std::back_inserter(response.m_data)); + std::string_view from{static_cast(buffer), data_length}; + + std::copy(from.begin(), from.end(), std::back_inserter(response.m_data)); return data_length; }