From 5e25b6d0b0b40ce9f40ddbe1e0339d6184cc2c33 Mon Sep 17 00:00:00 2001 From: Hind Montassif Date: Wed, 8 Feb 2023 17:54:10 +0100 Subject: [PATCH 1/9] Remove and move some functions --- include/powerloader/curl.hpp | 9 ++------- include/powerloader/url.hpp | 5 ----- src/curl.cpp | 5 ----- src/fastest_mirror.cpp | 6 +++--- src/target.cpp | 4 ++-- 5 files changed, 7 insertions(+), 22 deletions(-) diff --git a/include/powerloader/curl.hpp b/include/powerloader/curl.hpp index 3cbfb853..dd4b8d42 100644 --- a/include/powerloader/curl.hpp +++ b/include/powerloader/curl.hpp @@ -77,9 +77,6 @@ namespace powerloader nlohmann::json json() const; }; - // TODO: rename this, try to not expose it - POWERLOADER_API CURL* get_handle(const Context& ctx); - class POWERLOADER_API CURLHandle { public: @@ -94,15 +91,12 @@ namespace powerloader Response perform(); void finalize_transfer(); - // TODO: should be private? - void finalize_transfer(Response& response); template tl::expected getinfo(CURLINFO option); - // TODO: why do we need to expose these three methods + // TODO: why do we need to expose these methods CURL* handle(); - operator CURL*(); // TODO: consider making this `explicit` or remove it CURL* ptr() const; CURLHandle& add_header(const std::string& header); @@ -123,6 +117,7 @@ namespace powerloader private: void init_handle(const Context& ctx); + void finalize_transfer(Response& response); CURL* m_handle; curl_slist* p_headers = nullptr; diff --git a/include/powerloader/url.hpp b/include/powerloader/url.hpp index f78a6fc7..72daf406 100644 --- a/include/powerloader/url.hpp +++ b/include/powerloader/url.hpp @@ -135,11 +135,6 @@ namespace powerloader } } // namespace detail - inline std::string join_url() - { - return ""; - } - template inline std::string join_url(const S& s, const Args&... args) { diff --git a/src/curl.cpp b/src/curl.cpp index 368f9ffe..1e8eccc3 100644 --- a/src/curl.cpp +++ b/src/curl.cpp @@ -225,11 +225,6 @@ namespace powerloader return m_handle; } - CURLHandle::operator CURL*() - { - return handle(); - } - CURL* CURLHandle::ptr() const { return m_handle; diff --git a/src/fastest_mirror.cpp b/src/fastest_mirror.cpp index 6a3555dd..4dbc4291 100644 --- a/src/fastest_mirror.cpp +++ b/src/fastest_mirror.cpp @@ -54,9 +54,9 @@ namespace powerloader std::size_t handles_added = 0; for (auto& el : mirrors) { - if (el.handle) + if (el.handle.handle()) { - curl_multi_add_handle(multihandle, el.handle); + curl_multi_add_handle(multihandle, el.handle.handle()); handles_added++; spdlog::info("Checking URL: {}", el.url); } @@ -156,7 +156,7 @@ namespace powerloader for (auto& el : mirrors) { // Remove handle - curl_multi_remove_handle(multihandle, el.handle); + curl_multi_remove_handle(multihandle, el.handle.handle()); // Calculate plain_connect_time auto effective_url = el.handle.getinfo(CURLINFO_EFFECTIVE_URL); diff --git a/src/target.cpp b/src/target.cpp index 08b98ccd..e659ea0e 100644 --- a/src/target.cpp +++ b/src/target.cpp @@ -548,7 +548,7 @@ namespace powerloader m_mirror->prepare(m_target->path(), h); m_state = DownloadState::kPREPARATION; - CURLMcode cm_rc = curl_multi_add_handle(multi_handle, h); + CURLMcode cm_rc = curl_multi_add_handle(multi_handle, h.handle()); if (cm_rc != CURLM_OK) { spdlog::error("curl_multi_add_handle() failed: {}", curl_multi_strerror(cm_rc)); @@ -698,7 +698,7 @@ namespace powerloader } // Add the new handle to the curl multi handle - CURLMcode cm_rc = curl_multi_add_handle(multi_handle, h); + CURLMcode cm_rc = curl_multi_add_handle(multi_handle, h.handle()); if (cm_rc != CURLM_OK) { spdlog::error("curl_multi_add_handle() failed: {}", curl_multi_strerror(cm_rc)); From db1df51c38db79c2ee31065bcb5ee29dd63d848b Mon Sep 17 00:00:00 2001 From: Hind Montassif Date: Wed, 8 Feb 2023 18:24:40 +0100 Subject: [PATCH 2/9] Move enums (not related to curl) to enums header --- include/powerloader/download_target.hpp | 9 +- include/powerloader/enums.hpp | 119 ++++++++++++++++++++++++ include/powerloader/errors.hpp | 83 +---------------- include/powerloader/mirror.hpp | 10 -- include/powerloader/url.hpp | 21 ----- 5 files changed, 121 insertions(+), 121 deletions(-) diff --git a/include/powerloader/download_target.hpp b/include/powerloader/download_target.hpp index 50042eac..a018af4d 100644 --- a/include/powerloader/download_target.hpp +++ b/include/powerloader/download_target.hpp @@ -13,7 +13,6 @@ namespace powerloader { - struct zck_target; struct POWERLOADER_API CacheControl @@ -23,12 +22,6 @@ namespace powerloader std::string last_modified; }; - enum CompressionType - { - NONE, - ZSTD, - }; - class POWERLOADER_API DownloadTarget { public: @@ -80,7 +73,7 @@ namespace powerloader m_error = std::move(err); } - /// Returns a DownloadError if there was a falure at download or none if no error was set so + /// Returns a DownloadError if there was a failure at download or none if no error was set so /// far. std::optional get_error() const noexcept { diff --git a/include/powerloader/enums.hpp b/include/powerloader/enums.hpp index 8bfff40b..2130ac47 100644 --- a/include/powerloader/enums.hpp +++ b/include/powerloader/enums.hpp @@ -15,6 +15,22 @@ namespace powerloader // Want: S3, OCI }; + enum CompressionType + { + NONE, + ZSTD, + }; + + enum class MirrorState + { + WAITING, + AUTHENTICATING, + READY, + RETRY_DELAY, + AUTHENTICATION_FAILED, + FAILED + }; + enum class DownloadState { // The target is waiting to be processed. @@ -81,11 +97,114 @@ namespace powerloader kMD5 }; + /** Librepo return/error codes */ + enum class ErrorCode + { + // everything is ok + PD_OK, + // bad function argument + PD_BADFUNCARG, + // bad argument of the option + PD_BADOPTARG, + // library doesn't know the option + PD_UNKNOWNOPT, + // cURL doesn't know the option. Too old curl version? + PD_CURLSETOPT, + // Result object is not clean + PD_ADYUSEDRESULT, + // Result doesn't contain all what is needed + PD_INCOMPLETERESULT, + // cannot duplicate curl handle + PD_CURLDUP, + // cURL error + PD_CURL, + // cURL multi handle error + PD_CURLM, + // HTTP or FTP returned status code which do not represent success + // (file doesn't exists, etc.) + PD_BADSTATUS, + // some error that should be temporary and next try could work + // (HTTP status codes 500, 502-504, operation timeout, ...) + PD_TEMPORARYERR, + // URL is not a local address + PD_NOTLOCAL, + // cannot create a directory in output dir (ady exists?) + PD_CANNOTCREATEDIR, + // input output error + PD_IO, + // bad mirrorlist/metalink file (metalink doesn't contain needed + // file, mirrorlist doesn't contain urls, ..) + PD_MIRRORS, + // bad checksum + PD_BADCHECKSUM, + // no usable URL found + PD_NOURL, + // cannot create tmp directory + PD_CANNOTCREATETMP, + // unknown type of checksum is needed for verification + PD_UNKNOWNCHECKSUM, + // bad URL specified + PD_BADURL, + // Download was interrupted by signal. + // Only if LRO_INTERRUPTIBLE option is enabled. + PD_INTERRUPTED, + // sigaction error + PD_SIGACTION, + // File ady exists and checksum is ok.*/ + PD_ADYDOWNLOADED, + // The download wasn't or cannot be finished. + PD_UNFINISHED, + // select() call failed. + PD_SELECT, + // OpenSSL library related error. + PD_OPENSSL, + // Cannot allocate more memory + PD_MEMORY, + // Interrupted by user cb + PD_CBINTERRUPTED, + // File operation error (operation not permitted, filename too long, no memory available, + // bad file descriptor, ...) + PD_FILE, + // Zchunk error (error reading zchunk file, ...) + PD_ZCK, + // (xx) unknown error - sentinel of error codes enum + PD_UNKNOWNERROR, + }; + + enum ErrorLevel + { + INFO, + SERIOUS, + FATAL + }; + struct Checksum { ChecksumType type; std::string checksum; }; + + /** AS A REFERENCE */ + // typedef enum { + // CURLUE_OK, + // CURLUE_BAD_HANDLE, /* 1 */ + // CURLUE_BAD_PARTPOINTER, /* 2 */ + // CURLUE_MALFORMED_INPUT, /* 3 */ + // CURLUE_BAD_PORT_NUMBER, /* 4 */ + // CURLUE_UNSUPPORTED_SCHEME, /* 5 */ + // CURLUE_URLDECODE, /* 6 */ + // CURLUE_OUT_OF_MEMORY, /* 7 */ + // CURLUE_USER_NOT_ALLOWED, /* 8 */ + // CURLUE_UNKNOWN_PART, /* 9 */ + // CURLUE_NO_SCHEME, /* 10 */ + // CURLUE_NO_USER, /* 11 */ + // CURLUE_NO_PASSWORD, /* 12 */ + // CURLUE_NO_OPTIONS, /* 13 */ + // CURLUE_NO_HOST, /* 14 */ + // CURLUE_NO_PORT, /* 15 */ + // CURLUE_NO_QUERY, /* 16 */ + // CURLUE_NO_FRAGMENT /* 17 */ + // } CURLUcode; } #endif diff --git a/include/powerloader/errors.hpp b/include/powerloader/errors.hpp index f52e9d86..88cb3c19 100644 --- a/include/powerloader/errors.hpp +++ b/include/powerloader/errors.hpp @@ -2,91 +2,10 @@ #define POWERLOADER_ERRORS_HPP #include +#include namespace powerloader { - /** Librepo return/error codes - */ - enum class ErrorCode - { - // everything is ok - PD_OK, - // bad function argument - PD_BADFUNCARG, - // bad argument of the option - PD_BADOPTARG, - // library doesn't know the option - PD_UNKNOWNOPT, - // cURL doesn't know the option. Too old curl version? - PD_CURLSETOPT, - // Result object is not clean - PD_ADYUSEDRESULT, - // Result doesn't contain all what is needed - PD_INCOMPLETERESULT, - // cannot duplicate curl handle - PD_CURLDUP, - // cURL error - PD_CURL, - // cURL multi handle error - PD_CURLM, - // HTTP or FTP returned status code which do not represent success - // (file doesn't exists, etc.) - PD_BADSTATUS, - // some error that should be temporary and next try could work - // (HTTP status codes 500, 502-504, operation timeout, ...) - PD_TEMPORARYERR, - // URL is not a local address - PD_NOTLOCAL, - // cannot create a directory in output dir (ady exists?) - PD_CANNOTCREATEDIR, - // input output error - PD_IO, - // bad mirrorlist/metalink file (metalink doesn't contain needed - // file, mirrorlist doesn't contain urls, ..) - PD_MIRRORS, - // bad checksum - PD_BADCHECKSUM, - // no usable URL found - PD_NOURL, - // cannot create tmp directory - PD_CANNOTCREATETMP, - // unknown type of checksum is needed for verification - PD_UNKNOWNCHECKSUM, - // bad URL specified - PD_BADURL, - // Download was interrupted by signal. - // Only if LRO_INTERRUPTIBLE option is enabled. - PD_INTERRUPTED, - // sigaction error - PD_SIGACTION, - // File ady exists and checksum is ok.*/ - PD_ADYDOWNLOADED, - // The download wasn't or cannot be finished. - PD_UNFINISHED, - // select() call failed. - PD_SELECT, - // OpenSSL library related error. - PD_OPENSSL, - // Cannot allocate more memory - PD_MEMORY, - // Interrupted by user cb - PD_CBINTERRUPTED, - // File operation error (operation not permitted, filename too long, no memory available, - // bad file descriptor, ...) - PD_FILE, - // Zchunk error (error reading zchunk file, ...) - PD_ZCK, - // (xx) unknown error - sentinel of error codes enum - PD_UNKNOWNERROR, - }; - - enum ErrorLevel - { - INFO, - SERIOUS, - FATAL - }; - struct DownloaderError { ErrorLevel level; diff --git a/include/powerloader/mirror.hpp b/include/powerloader/mirror.hpp index 1679ef18..811bba1e 100644 --- a/include/powerloader/mirror.hpp +++ b/include/powerloader/mirror.hpp @@ -17,16 +17,6 @@ namespace powerloader class Target; class Context; - enum class MirrorState - { - WAITING, - AUTHENTICATING, - READY, - RETRY_DELAY, - AUTHENTICATION_FAILED, - FAILED - }; - struct MirrorStats { // Maximum number of allowed parallel connections to this mirror. -1 means no diff --git a/include/powerloader/url.hpp b/include/powerloader/url.hpp index 72daf406..56f7ef5c 100644 --- a/include/powerloader/url.hpp +++ b/include/powerloader/url.hpp @@ -19,27 +19,6 @@ extern "C" #include -// typedef enum { -// CURLUE_OK, -// CURLUE_BAD_HANDLE, /* 1 */ -// CURLUE_BAD_PARTPOINTER, /* 2 */ -// CURLUE_MALFORMED_INPUT, /* 3 */ -// CURLUE_BAD_PORT_NUMBER, /* 4 */ -// CURLUE_UNSUPPORTED_SCHEME, /* 5 */ -// CURLUE_URLDECODE, /* 6 */ -// CURLUE_OUT_OF_MEMORY, /* 7 */ -// CURLUE_USER_NOT_ALLOWED, /* 8 */ -// CURLUE_UNKNOWN_PART, /* 9 */ -// CURLUE_NO_SCHEME, /* 10 */ -// CURLUE_NO_USER, /* 11 */ -// CURLUE_NO_PASSWORD, /* 12 */ -// CURLUE_NO_OPTIONS, /* 13 */ -// CURLUE_NO_HOST, /* 14 */ -// CURLUE_NO_PORT, /* 15 */ -// CURLUE_NO_QUERY, /* 16 */ -// CURLUE_NO_FRAGMENT /* 17 */ -// } CURLUcode; - namespace powerloader { POWERLOADER_API bool has_scheme(const std::string& url); From 0a14e2ad3b5fdf44107c33de00a8f276f1d74217 Mon Sep 17 00:00:00 2001 From: Hind Montassif Date: Fri, 10 Feb 2023 16:10:14 +0100 Subject: [PATCH 3/9] Make callbacks for CURL handles private --- src/target.hpp | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/target.hpp b/src/target.hpp index b11db41e..9bdc71ef 100644 --- a/src/target.hpp +++ b/src/target.hpp @@ -22,21 +22,6 @@ namespace powerloader class Target { public: - /** Header callback for CURL handles. - * It parses HTTP and FTP headers and try to find length of the content - * (file size of the target). If the size is different then the expected - * size, then the transfer is interrupted. - * This callback is used only if the expected size is specified. - */ - static std::size_t header_callback(char* buffer, - std::size_t size, - std::size_t nitems, - Target* self); - static std::size_t write_callback(char* buffer, - std::size_t size, - std::size_t nitems, - Target* self); - Target(const Context& ctx, std::shared_ptr dl_target, mirror_set mirrors = {}); @@ -260,6 +245,21 @@ namespace powerloader std::size_t nitems, Target* self); + /** Header callback for CURL handles. + * It parses HTTP and FTP headers and try to find length of the content + * (file size of the target). If the size is different then the expected + * size, then the transfer is interrupted. + * This callback is used only if the expected size is specified. + */ + static std::size_t header_callback(char* buffer, + std::size_t size, + std::size_t nitems, + Target* self); + static std::size_t write_callback(char* buffer, + std::size_t size, + std::size_t nitems, + Target* self); + #ifdef WITH_ZSTD std::unique_ptr m_zstd_stream; #endif From 002e3422d5aa55f1376e841e4c7f03b1cbf80715 Mon Sep 17 00:00:00 2001 From: Hind Montassif Date: Mon, 13 Feb 2023 14:53:23 +0100 Subject: [PATCH 4/9] Remove ptr func in CURLHandle --- include/powerloader/curl.hpp | 1 - src/curl.cpp | 4 ---- src/target.cpp | 4 +--- 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/include/powerloader/curl.hpp b/include/powerloader/curl.hpp index dd4b8d42..cb8bcd26 100644 --- a/include/powerloader/curl.hpp +++ b/include/powerloader/curl.hpp @@ -97,7 +97,6 @@ namespace powerloader // TODO: why do we need to expose these methods CURL* handle(); - CURL* ptr() const; CURLHandle& add_header(const std::string& header); CURLHandle& add_headers(const std::vector& headers); diff --git a/src/curl.cpp b/src/curl.cpp index 1e8eccc3..8141893e 100644 --- a/src/curl.cpp +++ b/src/curl.cpp @@ -225,10 +225,6 @@ namespace powerloader return m_handle; } - CURL* CURLHandle::ptr() const - { - return m_handle; - } CURLHandle& CURLHandle::add_header(const std::string& header) { diff --git a/src/target.cpp b/src/target.cpp index e659ea0e..d2aefcc8 100644 --- a/src/target.cpp +++ b/src/target.cpp @@ -169,9 +169,7 @@ namespace powerloader std::size_t zckheadercb(char* buffer, std::size_t size, std::size_t nitems, Target* self) { assert(self && self->m_target); - long code = -1; - curl_easy_getinfo(self->m_curl_handle->ptr(), CURLINFO_RESPONSE_CODE, &code); - if (code == 200) + if (self->m_curl_handle->getinfo(CURLINFO_RESPONSE_CODE) == 200) { spdlog::info("Too many ranges were attempted in one download"); self->m_range_fail = 1; From 162771cae4fba3f8e1da17b2c88f6ac1a2757e87 Mon Sep 17 00:00:00 2001 From: Hind Montassif Date: Mon, 13 Feb 2023 15:11:36 +0100 Subject: [PATCH 5/9] Add interface to wrap libcurl handle usage --- include/powerloader/curl.hpp | 10 ++++++++-- src/curl.cpp | 18 ++++++++++++++++++ src/curl_internal.hpp | 18 ++++++++++++++++++ src/downloader.cpp | 3 ++- src/fastest_mirror.cpp | 8 +++++--- src/target.cpp | 5 +++-- 6 files changed, 54 insertions(+), 8 deletions(-) diff --git a/include/powerloader/curl.hpp b/include/powerloader/curl.hpp index cb8bcd26..aaa0f9af 100644 --- a/include/powerloader/curl.hpp +++ b/include/powerloader/curl.hpp @@ -95,8 +95,7 @@ namespace powerloader template tl::expected getinfo(CURLINFO option); - // TODO: why do we need to expose these methods - CURL* handle(); + bool handle_exists(); CURLHandle& add_header(const std::string& header); CURLHandle& add_headers(const std::vector& headers); @@ -117,6 +116,7 @@ namespace powerloader private: void init_handle(const Context& ctx); void finalize_transfer(Response& response); + CURL* handle(); CURL* m_handle; curl_slist* p_headers = nullptr; @@ -124,6 +124,12 @@ namespace powerloader std::unique_ptr response; end_callback_type end_callback; + + friend class CURLInterface; + + friend void add_multipart_upload(CURLHandle& target, + const std::vector& files, + const std::map& extra_fields); }; // TODO: restrict the possible implementations in the cpp file diff --git a/src/curl.cpp b/src/curl.cpp index 8141893e..e90f87eb 100644 --- a/src/curl.cpp +++ b/src/curl.cpp @@ -225,6 +225,10 @@ namespace powerloader return m_handle; } + bool CURLHandle::handle_exists() + { + return (handle() != nullptr); + } CURLHandle& CURLHandle::add_header(const std::string& header) { @@ -463,6 +467,20 @@ namespace powerloader curl_global_cleanup(); is_curl_setup_alive = false; } + } + + CURLMcode CURLInterface::multi_add_handle(CURLM* multi_handle, CURLHandle& h) + { + return curl_multi_add_handle(multi_handle, h.handle()); + } + void CURLInterface::multi_remove_handle(CURLM* multihandle, CURLHandle& h) + { + curl_multi_remove_handle(multihandle, h.handle()); + } + + bool CURLInterface::handle_is_equal(CURLHandle* h, CURLMsg* msg) + { + return (h->handle() == msg->easy_handle); } } diff --git a/src/curl_internal.hpp b/src/curl_internal.hpp index a15a9af9..8f36a491 100644 --- a/src/curl_internal.hpp +++ b/src/curl_internal.hpp @@ -23,4 +23,22 @@ namespace powerloader::details CURLSetup& operator=(const CURLSetup&) = delete; }; } + +namespace powerloader +{ + class CURLInterface + { + public: + CURLInterface() = delete; + ~CURLInterface() = delete; + + static CURLMcode multi_add_handle(CURLM* multi_handle, + CURLHandle& h); + + static void multi_remove_handle(CURLM* multihandle, + CURLHandle& h); + + static bool handle_is_equal(CURLHandle* h, CURLMsg* msg); + }; +} #endif diff --git a/src/downloader.cpp b/src/downloader.cpp index 1a181543..b1cd5399 100644 --- a/src/downloader.cpp +++ b/src/downloader.cpp @@ -23,6 +23,7 @@ namespace fs = std::filesystem; #include #include #include "target.hpp" +#include "curl_internal.hpp" #ifdef WITH_ZCHUNK #include "zck.hpp" #endif @@ -514,7 +515,7 @@ namespace powerloader for (auto* target : m_running_transfers) { - if (target->curl_handle() && target->curl_handle()->handle() == msg->easy_handle) + if (target->curl_handle() && CURLInterface::handle_is_equal(target->curl_handle(), msg)) { current_target = target; break; diff --git a/src/fastest_mirror.cpp b/src/fastest_mirror.cpp index 4dbc4291..9903d9e5 100644 --- a/src/fastest_mirror.cpp +++ b/src/fastest_mirror.cpp @@ -6,6 +6,8 @@ #include #include +#include "curl_internal.hpp" + namespace powerloader { namespace detail @@ -54,9 +56,9 @@ namespace powerloader std::size_t handles_added = 0; for (auto& el : mirrors) { - if (el.handle.handle()) + if (el.handle.handle_exists()) { - curl_multi_add_handle(multihandle, el.handle.handle()); + CURLInterface::multi_add_handle(multihandle, el.handle); handles_added++; spdlog::info("Checking URL: {}", el.url); } @@ -156,7 +158,7 @@ namespace powerloader for (auto& el : mirrors) { // Remove handle - curl_multi_remove_handle(multihandle, el.handle.handle()); + CURLInterface::multi_remove_handle(multihandle, el.handle); // Calculate plain_connect_time auto effective_url = el.handle.getinfo(CURLINFO_EFFECTIVE_URL); diff --git a/src/target.cpp b/src/target.cpp index d2aefcc8..9db3514f 100644 --- a/src/target.cpp +++ b/src/target.cpp @@ -1,4 +1,5 @@ #include "target.hpp" +#include "curl_internal.hpp" #ifdef WITH_ZCHUNK #include "zck.hpp" @@ -546,7 +547,7 @@ namespace powerloader m_mirror->prepare(m_target->path(), h); m_state = DownloadState::kPREPARATION; - CURLMcode cm_rc = curl_multi_add_handle(multi_handle, h.handle()); + CURLMcode cm_rc = CURLInterface::multi_add_handle(multi_handle, h); if (cm_rc != CURLM_OK) { spdlog::error("curl_multi_add_handle() failed: {}", curl_multi_strerror(cm_rc)); @@ -696,7 +697,7 @@ namespace powerloader } // Add the new handle to the curl multi handle - CURLMcode cm_rc = curl_multi_add_handle(multi_handle, h.handle()); + CURLMcode cm_rc = CURLInterface::multi_add_handle(multi_handle, h); if (cm_rc != CURLM_OK) { spdlog::error("curl_multi_add_handle() failed: {}", curl_multi_strerror(cm_rc)); From ac9784c8b586cc439addf72ac037b55d4af35cd5 Mon Sep 17 00:00:00 2001 From: Hind Montassif Date: Tue, 14 Feb 2023 10:36:46 +0100 Subject: [PATCH 6/9] Wrap getinfo --- include/powerloader/curl.hpp | 7 ++++--- src/curl.cpp | 14 ++++++++++---- src/curl_internal.hpp | 3 +++ src/fastest_mirror.cpp | 6 +++--- src/target.cpp | 4 ++-- 5 files changed, 22 insertions(+), 12 deletions(-) diff --git a/include/powerloader/curl.hpp b/include/powerloader/curl.hpp index aaa0f9af..e8aab0f9 100644 --- a/include/powerloader/curl.hpp +++ b/include/powerloader/curl.hpp @@ -92,9 +92,6 @@ namespace powerloader Response perform(); void finalize_transfer(); - template - tl::expected getinfo(CURLINFO option); - bool handle_exists(); CURLHandle& add_header(const std::string& header); @@ -116,8 +113,12 @@ namespace powerloader private: void init_handle(const Context& ctx); void finalize_transfer(Response& response); + CURL* handle(); + template + tl::expected getinfo(CURLINFO option); + CURL* m_handle; curl_slist* p_headers = nullptr; char errorbuffer[CURL_ERROR_SIZE]; diff --git a/src/curl.cpp b/src/curl.cpp index e90f87eb..1353b7c0 100644 --- a/src/curl.cpp +++ b/src/curl.cpp @@ -382,11 +382,11 @@ namespace powerloader void Response::fill_values(CURLHandle& handle) { average_speed - = handle.getinfo(CURLINFO_SPEED_DOWNLOAD_T).value_or(0); - http_status = handle.getinfo(CURLINFO_RESPONSE_CODE).value(); - effective_url = handle.getinfo(CURLINFO_EFFECTIVE_URL).value(); + = CURLInterface::get_info_wrapped(handle, CURLINFO_SPEED_DOWNLOAD_T).value_or(0); + http_status = CURLInterface::get_info_wrapped(handle, CURLINFO_RESPONSE_CODE).value(); + effective_url = CURLInterface::get_info_wrapped(handle, CURLINFO_EFFECTIVE_URL).value(); downloaded_size - = handle.getinfo(CURLINFO_SIZE_DOWNLOAD_T).value(); + = CURLInterface::get_info_wrapped(handle, CURLINFO_SIZE_DOWNLOAD_T).value(); } std::optional proxy_match(const proxy_map_type& proxies, const std::string& url) @@ -483,4 +483,10 @@ namespace powerloader { return (h->handle() == msg->easy_handle); } + + template + tl::expected CURLInterface::get_info_wrapped(CURLHandle& h, CURLINFO option) + { + return h.getinfo(option); + } } diff --git a/src/curl_internal.hpp b/src/curl_internal.hpp index 8f36a491..02ce0c1c 100644 --- a/src/curl_internal.hpp +++ b/src/curl_internal.hpp @@ -39,6 +39,9 @@ namespace powerloader CURLHandle& h); static bool handle_is_equal(CURLHandle* h, CURLMsg* msg); + + template + static tl::expected get_info_wrapped(CURLHandle& h, CURLINFO option); }; } #endif diff --git a/src/fastest_mirror.cpp b/src/fastest_mirror.cpp index 9903d9e5..664106d1 100644 --- a/src/fastest_mirror.cpp +++ b/src/fastest_mirror.cpp @@ -161,7 +161,7 @@ namespace powerloader CURLInterface::multi_remove_handle(multihandle, el.handle); // Calculate plain_connect_time - auto effective_url = el.handle.getinfo(CURLINFO_EFFECTIVE_URL); + auto effective_url = CURLInterface::get_info_wrapped(el.handle, CURLINFO_EFFECTIVE_URL); if (!effective_url) { @@ -177,9 +177,9 @@ namespace powerloader { // Get connect time curl_off_t namelookup_time - = el.handle.getinfo(CURLINFO_NAMELOOKUP_TIME_T).value_or(0); + = CURLInterface::get_info_wrapped(el.handle, CURLINFO_NAMELOOKUP_TIME_T).value_or(0); curl_off_t connect_time - = el.handle.getinfo(CURLINFO_CONNECT_TIME_T).value_or(0); + = CURLInterface::get_info_wrapped(el.handle, CURLINFO_CONNECT_TIME_T).value_or(0); if (connect_time == 0) { diff --git a/src/target.cpp b/src/target.cpp index 9db3514f..f06cb1e3 100644 --- a/src/target.cpp +++ b/src/target.cpp @@ -64,7 +64,7 @@ namespace powerloader if (!ec && m_ctx.preserve_filetime) { - auto remote_filetime = m_curl_handle->getinfo(CURLINFO_FILETIME_T); + auto remote_filetime = CURLInterface::get_info_wrapped(*m_curl_handle, CURLINFO_FILETIME_T); if (!remote_filetime || remote_filetime.value() < 0) spdlog::debug("Unable to get remote time of retrieved document"); @@ -170,7 +170,7 @@ namespace powerloader std::size_t zckheadercb(char* buffer, std::size_t size, std::size_t nitems, Target* self) { assert(self && self->m_target); - if (self->m_curl_handle->getinfo(CURLINFO_RESPONSE_CODE) == 200) + if (CURLInterface::get_info_wrapped(*(self->m_curl_handle), CURLINFO_RESPONSE_CODE) == 200) { spdlog::info("Too many ranges were attempted in one download"); self->m_range_fail = 1; From 6820a29e49ef905439908ef7e88cbe35a20be025 Mon Sep 17 00:00:00 2001 From: Hind Montassif Date: Tue, 14 Feb 2023 10:57:09 +0100 Subject: [PATCH 7/9] Fix linter --- include/powerloader/curl.hpp | 4 ++-- include/powerloader/download_target.hpp | 4 ++-- src/curl.cpp | 18 ++++++++++++------ src/curl_internal.hpp | 6 ++---- src/downloader.cpp | 3 ++- src/fastest_mirror.cpp | 13 ++++++++----- src/target.cpp | 6 ++++-- 7 files changed, 32 insertions(+), 22 deletions(-) diff --git a/include/powerloader/curl.hpp b/include/powerloader/curl.hpp index e8aab0f9..b396a35b 100644 --- a/include/powerloader/curl.hpp +++ b/include/powerloader/curl.hpp @@ -129,8 +129,8 @@ namespace powerloader friend class CURLInterface; friend void add_multipart_upload(CURLHandle& target, - const std::vector& files, - const std::map& extra_fields); + const std::vector& files, + const std::map& extra_fields); }; // TODO: restrict the possible implementations in the cpp file diff --git a/include/powerloader/download_target.hpp b/include/powerloader/download_target.hpp index a018af4d..7e6a18c4 100644 --- a/include/powerloader/download_target.hpp +++ b/include/powerloader/download_target.hpp @@ -73,8 +73,8 @@ namespace powerloader m_error = std::move(err); } - /// Returns a DownloadError if there was a failure at download or none if no error was set so - /// far. + /// Returns a DownloadError if there was a failure at download or none if no error was set + /// so far. std::optional get_error() const noexcept { return m_error; diff --git a/src/curl.cpp b/src/curl.cpp index 1353b7c0..040bdc2a 100644 --- a/src/curl.cpp +++ b/src/curl.cpp @@ -381,12 +381,18 @@ namespace powerloader void Response::fill_values(CURLHandle& handle) { - average_speed - = CURLInterface::get_info_wrapped(handle, CURLINFO_SPEED_DOWNLOAD_T).value_or(0); - http_status = CURLInterface::get_info_wrapped(handle, CURLINFO_RESPONSE_CODE).value(); - effective_url = CURLInterface::get_info_wrapped(handle, CURLINFO_EFFECTIVE_URL).value(); - downloaded_size - = CURLInterface::get_info_wrapped(handle, CURLINFO_SIZE_DOWNLOAD_T).value(); + average_speed = CURLInterface::get_info_wrapped( + handle, CURLINFO_SPEED_DOWNLOAD_T) + .value_or(0); + http_status + = CURLInterface::get_info_wrapped(handle, CURLINFO_RESPONSE_CODE) + .value(); + effective_url = CURLInterface::get_info_wrapped( + handle, CURLINFO_EFFECTIVE_URL) + .value(); + downloaded_size = CURLInterface::get_info_wrapped( + handle, CURLINFO_SIZE_DOWNLOAD_T) + .value(); } std::optional proxy_match(const proxy_map_type& proxies, const std::string& url) diff --git a/src/curl_internal.hpp b/src/curl_internal.hpp index 02ce0c1c..b7a75937 100644 --- a/src/curl_internal.hpp +++ b/src/curl_internal.hpp @@ -32,11 +32,9 @@ namespace powerloader CURLInterface() = delete; ~CURLInterface() = delete; - static CURLMcode multi_add_handle(CURLM* multi_handle, - CURLHandle& h); + static CURLMcode multi_add_handle(CURLM* multi_handle, CURLHandle& h); - static void multi_remove_handle(CURLM* multihandle, - CURLHandle& h); + static void multi_remove_handle(CURLM* multihandle, CURLHandle& h); static bool handle_is_equal(CURLHandle* h, CURLMsg* msg); diff --git a/src/downloader.cpp b/src/downloader.cpp index b1cd5399..457e27b3 100644 --- a/src/downloader.cpp +++ b/src/downloader.cpp @@ -515,7 +515,8 @@ namespace powerloader for (auto* target : m_running_transfers) { - if (target->curl_handle() && CURLInterface::handle_is_equal(target->curl_handle(), msg)) + if (target->curl_handle() + && CURLInterface::handle_is_equal(target->curl_handle(), msg)) { current_target = target; break; diff --git a/src/fastest_mirror.cpp b/src/fastest_mirror.cpp index 664106d1..9f7881d3 100644 --- a/src/fastest_mirror.cpp +++ b/src/fastest_mirror.cpp @@ -161,7 +161,8 @@ namespace powerloader CURLInterface::multi_remove_handle(multihandle, el.handle); // Calculate plain_connect_time - auto effective_url = CURLInterface::get_info_wrapped(el.handle, CURLINFO_EFFECTIVE_URL); + auto effective_url = CURLInterface::get_info_wrapped( + el.handle, CURLINFO_EFFECTIVE_URL); if (!effective_url) { @@ -176,10 +177,12 @@ namespace powerloader else { // Get connect time - curl_off_t namelookup_time - = CURLInterface::get_info_wrapped(el.handle, CURLINFO_NAMELOOKUP_TIME_T).value_or(0); - curl_off_t connect_time - = CURLInterface::get_info_wrapped(el.handle, CURLINFO_CONNECT_TIME_T).value_or(0); + curl_off_t namelookup_time = CURLInterface::get_info_wrapped( + el.handle, CURLINFO_NAMELOOKUP_TIME_T) + .value_or(0); + curl_off_t connect_time = CURLInterface::get_info_wrapped( + el.handle, CURLINFO_CONNECT_TIME_T) + .value_or(0); if (connect_time == 0) { diff --git a/src/target.cpp b/src/target.cpp index f06cb1e3..68d252cc 100644 --- a/src/target.cpp +++ b/src/target.cpp @@ -64,7 +64,8 @@ namespace powerloader if (!ec && m_ctx.preserve_filetime) { - auto remote_filetime = CURLInterface::get_info_wrapped(*m_curl_handle, CURLINFO_FILETIME_T); + auto remote_filetime = CURLInterface::get_info_wrapped( + *m_curl_handle, CURLINFO_FILETIME_T); if (!remote_filetime || remote_filetime.value() < 0) spdlog::debug("Unable to get remote time of retrieved document"); @@ -170,7 +171,8 @@ namespace powerloader std::size_t zckheadercb(char* buffer, std::size_t size, std::size_t nitems, Target* self) { assert(self && self->m_target); - if (CURLInterface::get_info_wrapped(*(self->m_curl_handle), CURLINFO_RESPONSE_CODE) == 200) + if (CURLInterface::get_info_wrapped(*(self->m_curl_handle), CURLINFO_RESPONSE_CODE) + == 200) { spdlog::info("Too many ranges were attempted in one download"); self->m_range_fail = 1; From 902c9673c6555f217636edbb8203be040f9aed48 Mon Sep 17 00:00:00 2001 From: Hind Montassif Date: Tue, 14 Feb 2023 11:36:41 +0100 Subject: [PATCH 8/9] Specify get_info_wrapped implementations in cpp file --- src/curl.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/curl.cpp b/src/curl.cpp index 040bdc2a..734841e3 100644 --- a/src/curl.cpp +++ b/src/curl.cpp @@ -495,4 +495,17 @@ namespace powerloader { return h.getinfo(option); } + + template tl::expected CURLInterface::get_info_wrapped(CURLHandle& h, + CURLINFO option); + template tl::expected CURLInterface::get_info_wrapped(CURLHandle& h, + CURLINFO option); + template tl::expected CURLInterface::get_info_wrapped(CURLHandle& h, + CURLINFO option); + template tl::expected CURLInterface::get_info_wrapped(CURLHandle& h, + CURLINFO option); + template tl::expected CURLInterface::get_info_wrapped(CURLHandle& h, + CURLINFO option); + template tl::expected CURLInterface::get_info_wrapped(CURLHandle& h, + CURLINFO option); } From a3b2703e962392ad98c4c15df3519c0be204d7bf Mon Sep 17 00:00:00 2001 From: Hind Montassif Date: Wed, 15 Feb 2023 10:03:34 +0100 Subject: [PATCH 9/9] Wrap setopt function --- include/powerloader/curl.hpp | 6 +++--- src/curl.cpp | 8 ++++---- src/curl_internal.hpp | 9 +++++++++ src/fastest_mirror.cpp | 2 +- src/mirror.cpp | 6 +++--- src/mirrors/oci.cpp | 5 +++-- src/target.cpp | 33 ++++++++++++++++++--------------- src/uploader/oci_upload.cpp | 10 +++++----- src/uploader/s3_upload.cpp | 3 ++- 9 files changed, 48 insertions(+), 34 deletions(-) diff --git a/include/powerloader/curl.hpp b/include/powerloader/curl.hpp index b396a35b..2f533b17 100644 --- a/include/powerloader/curl.hpp +++ b/include/powerloader/curl.hpp @@ -98,9 +98,6 @@ namespace powerloader CURLHandle& add_headers(const std::vector& headers); CURLHandle& reset_headers(); - template - CURLHandle& setopt(CURLoption opt, const T& val); - void set_default_callbacks(); CURLHandle& set_end_callback(end_callback_type func); @@ -119,6 +116,9 @@ namespace powerloader template tl::expected getinfo(CURLINFO option); + template + CURLHandle& setopt(CURLoption opt, const T& val); + CURL* m_handle; curl_slist* p_headers = nullptr; char errorbuffer[CURL_ERROR_SIZE]; diff --git a/src/curl.cpp b/src/curl.cpp index 734841e3..e4db5cb5 100644 --- a/src/curl.cpp +++ b/src/curl.cpp @@ -325,12 +325,12 @@ namespace powerloader if (fsize != -1) { - curl.setopt(CURLOPT_INFILESIZE_LARGE, fsize); + CURLInterface::set_opt_wrapped(curl, CURLOPT_INFILESIZE_LARGE, fsize); } - curl.setopt(CURLOPT_UPLOAD, 1L); - curl.setopt(CURLOPT_READFUNCTION, read_callback); - curl.setopt(CURLOPT_READDATA, &stream); + CURLInterface::set_opt_wrapped(curl, CURLOPT_UPLOAD, 1L); + CURLInterface::set_opt_wrapped(curl, CURLOPT_READFUNCTION, read_callback); + CURLInterface::set_opt_wrapped(curl, CURLOPT_READDATA, &stream); return curl; } } diff --git a/src/curl_internal.hpp b/src/curl_internal.hpp index b7a75937..5e9353d7 100644 --- a/src/curl_internal.hpp +++ b/src/curl_internal.hpp @@ -40,6 +40,15 @@ namespace powerloader template static tl::expected get_info_wrapped(CURLHandle& h, CURLINFO option); + + template + static CURLHandle& set_opt_wrapped(CURLHandle& h, CURLoption opt, const T& val); }; + + template + CURLHandle& CURLInterface::set_opt_wrapped(CURLHandle& h, CURLoption opt, const T& val) + { + return h.setopt(opt, val); + } } #endif diff --git a/src/fastest_mirror.cpp b/src/fastest_mirror.cpp index 9f7881d3..5ef46019 100644 --- a/src/fastest_mirror.cpp +++ b/src/fastest_mirror.cpp @@ -32,7 +32,7 @@ namespace powerloader for (const std::string& u : urls) { CURLHandle handle(ctx, u); - handle.setopt(CURLOPT_CONNECT_ONLY, 1L); + CURLInterface::set_opt_wrapped(handle, CURLOPT_CONNECT_ONLY, 1L); check_mirrors.push_back(detail::InternalMirror{ u, std::move(handle), -1 }); } return fastestmirror_perform(check_mirrors, 1000000); diff --git a/src/mirror.cpp b/src/mirror.cpp index db60e933..6aaa6fb1 100644 --- a/src/mirror.cpp +++ b/src/mirror.cpp @@ -7,7 +7,7 @@ #include #include "powerloader/mirrorid.hpp" #include "target.hpp" - +#include "curl_internal.hpp" namespace powerloader { @@ -232,8 +232,8 @@ namespace powerloader { spdlog::warn( "Setting HTTP authentication for {} to {}:{}", path, m_auth_user, m_auth_password); - handle.setopt(CURLOPT_USERNAME, m_auth_user.c_str()); - handle.setopt(CURLOPT_PASSWORD, m_auth_password.c_str()); + CURLInterface::set_opt_wrapped(handle, CURLOPT_USERNAME, m_auth_user.c_str()); + CURLInterface::set_opt_wrapped(handle, CURLOPT_PASSWORD, m_auth_password.c_str()); } return true; } diff --git a/src/mirrors/oci.cpp b/src/mirrors/oci.cpp index c887a93f..892a34de 100644 --- a/src/mirrors/oci.cpp +++ b/src/mirrors/oci.cpp @@ -2,6 +2,7 @@ #include #include "target.hpp" +#include "curl_internal.hpp" namespace powerloader { @@ -151,11 +152,11 @@ namespace powerloader if (!m_username.empty()) { - handle.setopt(CURLOPT_USERNAME, m_username.c_str()); + CURLInterface::set_opt_wrapped(handle, CURLOPT_USERNAME, m_username.c_str()); } if (!m_password.empty()) { - handle.setopt(CURLOPT_PASSWORD, m_password.c_str()); + CURLInterface::set_opt_wrapped(handle, CURLOPT_PASSWORD, m_password.c_str()); } auto end_callback = [&cbdata](const Response& response) diff --git a/src/target.cpp b/src/target.cpp index 68d252cc..348f61d2 100644 --- a/src/target.cpp +++ b/src/target.cpp @@ -527,8 +527,9 @@ namespace powerloader { assert(m_curl_handle); assert(m_ctx.max_speed_limit > 0); - m_curl_handle->setopt(CURLOPT_MAX_RECV_SPEED_LARGE, - static_cast(m_ctx.max_speed_limit)); + CURLInterface::set_opt_wrapped(*m_curl_handle, + CURLOPT_MAX_RECV_SPEED_LARGE, + static_cast(m_ctx.max_speed_limit)); } void Target::reset_response() @@ -565,7 +566,7 @@ namespace powerloader if (m_target->head_only()) { - h.setopt(CURLOPT_NOBODY, true); + CURLInterface::set_opt_wrapped(h, CURLOPT_NOBODY, true); } // Prepare FILE #ifdef WITH_ZCHUNK @@ -631,33 +632,34 @@ namespace powerloader curl_off_t used_offset = m_original_offset; spdlog::info("Trying to resume from offset {}", used_offset); - h.setopt(CURLOPT_RESUME_FROM_LARGE, used_offset); + CURLInterface::set_opt_wrapped(h, CURLOPT_RESUME_FROM_LARGE, used_offset); } if (m_target->byterange_start() > 0) { assert(!m_target->resume() && m_target->range().empty()); - h.setopt(CURLOPT_RESUME_FROM_LARGE, (curl_off_t) m_target->byterange_start()); + CURLInterface::set_opt_wrapped( + h, CURLOPT_RESUME_FROM_LARGE, (curl_off_t) m_target->byterange_start()); } // Set range if user specified one if (!m_target->range().empty()) { assert(!m_target->resume() && !m_target->byterange_start()); - h.setopt(CURLOPT_RANGE, m_target->range()); + CURLInterface::set_opt_wrapped(h, CURLOPT_RANGE, m_target->range()); } // Prepare progress callback if (m_target->progress_callback()) { - h.setopt(CURLOPT_XFERINFOFUNCTION, &Target::progress_callback); - h.setopt(CURLOPT_NOPROGRESS, 0); - h.setopt(CURLOPT_XFERINFODATA, this); + CURLInterface::set_opt_wrapped(h, CURLOPT_XFERINFOFUNCTION, &Target::progress_callback); + CURLInterface::set_opt_wrapped(h, CURLOPT_NOPROGRESS, 0); + CURLInterface::set_opt_wrapped(h, CURLOPT_XFERINFODATA, this); } // Prepare header callback - h.setopt(CURLOPT_HEADERFUNCTION, &Target::header_callback); - h.setopt(CURLOPT_HEADERDATA, this); + CURLInterface::set_opt_wrapped(h, CURLOPT_HEADERFUNCTION, &Target::header_callback); + CURLInterface::set_opt_wrapped(h, CURLOPT_HEADERDATA, this); if (!m_target->head_only()) { @@ -665,15 +667,16 @@ namespace powerloader { #ifdef WITH_ZSTD m_zstd_stream = std::make_unique(&Target::write_callback, this); - h.setopt(CURLOPT_WRITEFUNCTION, &ZstdStream::write_callback); - h.setopt(CURLOPT_WRITEDATA, m_zstd_stream.get()); + CURLInterface::set_opt_wrapped( + h, CURLOPT_WRITEFUNCTION, &ZstdStream::write_callback); + CURLInterface::set_opt_wrapped(h, CURLOPT_WRITEDATA, m_zstd_stream.get()); #endif } // Prepare write callback else { - h.setopt(CURLOPT_WRITEFUNCTION, &Target::write_callback); - h.setopt(CURLOPT_WRITEDATA, this); + CURLInterface::set_opt_wrapped(h, CURLOPT_WRITEFUNCTION, &Target::write_callback); + CURLInterface::set_opt_wrapped(h, CURLOPT_WRITEDATA, this); } } diff --git a/src/uploader/oci_upload.cpp b/src/uploader/oci_upload.cpp index 73575f1f..097f60d8 100644 --- a/src/uploader/oci_upload.cpp +++ b/src/uploader/oci_upload.cpp @@ -1,10 +1,10 @@ -#include - #include #include #include +#include +#include "curl_internal.hpp" namespace powerloader { @@ -91,8 +91,8 @@ namespace powerloader const std::string& reference) const { std::string preupload_url = mirror.get_preupload_url(reference); - auto response = CURLHandle(ctx, preupload_url) - .setopt(CURLOPT_CUSTOMREQUEST, "POST") + CURLHandle handle(ctx, preupload_url); + auto response = CURLInterface::set_opt_wrapped(handle, CURLOPT_CUSTOMREQUEST, "POST") .add_headers(mirror.get_auth_headers(reference)) .perform(); @@ -109,7 +109,7 @@ namespace powerloader CURLHandle chandle(ctx, upload_url); // for uploading we always use application/octet-stream. The proper mimetypes // are defined in the manifest - chandle.setopt(CURLOPT_UPLOAD, 1L) + CURLInterface::set_opt_wrapped(chandle, CURLOPT_UPLOAD, 1L) .add_headers(mirror.get_auth_headers(reference)) .add_header(fmt::format("Content-Type: application/octet-stream", mime_type)); diff --git a/src/uploader/s3_upload.cpp b/src/uploader/s3_upload.cpp index 21bcd0cb..9dc22b0a 100644 --- a/src/uploader/s3_upload.cpp +++ b/src/uploader/s3_upload.cpp @@ -2,6 +2,7 @@ #include #include +#include "curl_internal.hpp" namespace powerloader { @@ -25,7 +26,7 @@ namespace powerloader std::ifstream ufile(file, std::ios::in | std::ios::binary); - uploadrequest.setopt(CURLOPT_INFILESIZE_LARGE, fsize) + CURLInterface::set_opt_wrapped(uploadrequest, CURLOPT_INFILESIZE_LARGE, fsize) .add_headers(mirror.get_auth_headers(request)) .upload(ufile);