From 297a8bc2c34db79598d56d53ae790eb9f72e9d2e Mon Sep 17 00:00:00 2001 From: Andrew Gasparovic <72571446+agasparovic@users.noreply.github.com> Date: Sat, 10 Oct 2020 20:03:49 -0700 Subject: [PATCH 1/2] Use move semantics instead of copy for functions In some cases, a few more copies could be prevented by changing function definitions to accept parameters by const-ref, rather than by value, but I didn't want to change public signatures. --- httplib.h | 157 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 93 insertions(+), 64 deletions(-) diff --git a/httplib.h b/httplib.h index 5139b7f05a..2039f5b198 100644 --- a/httplib.h +++ b/httplib.h @@ -317,14 +317,17 @@ class ContentReader { ContentReceiver receiver)>; ContentReader(Reader reader, MultipartReader multipart_reader) - : reader_(reader), multipart_reader_(multipart_reader) {} + : reader_(std::move(reader)), + multipart_reader_(std::move(multipart_reader)) {} bool operator()(MultipartContentHeader header, ContentReceiver receiver) const { - return multipart_reader_(header, receiver); + return multipart_reader_(std::move(header), std::move(receiver)); } - bool operator()(ContentReceiver receiver) const { return reader_(receiver); } + bool operator()(ContentReceiver receiver) const { + return reader_(std::move(receiver)); + } Reader reader_; MultipartReader multipart_reader_; @@ -475,7 +478,7 @@ class ThreadPool : public TaskQueue { void enqueue(std::function fn) override { std::unique_lock lock(mutex_); - jobs_.push_back(fn); + jobs_.push_back(std::move(fn)); cond_.notify_one(); } @@ -1944,7 +1947,7 @@ inline socket_t create_client_socket(const char *host, int port, time_t timeout_sec, time_t timeout_usec, const std::string &intf, Error &error) { auto sock = create_socket( - host, port, 0, tcp_nodelay, socket_options, + host, port, 0, tcp_nodelay, std::move(socket_options), [&](socket_t sock, struct addrinfo &ai) -> bool { if (!intf.empty()) { #ifdef USE_IF2IP @@ -2544,7 +2547,8 @@ inline bool read_content_chunked(Stream &strm, ContentReceiver out) { if (chunk_len == 0) { break; } - if (!read_content_with_length(strm, chunk_len, nullptr, out)) { + if (!read_content_with_length(strm, chunk_len, nullptr, + std::move(out))) { return false; } @@ -2600,7 +2604,7 @@ bool prepare_content_receiver(T &x, int &status, ContentReceiver receiver, buf, n, [&](const char *buf, size_t n) { return receiver(buf, n); }); }; - return callback(out); + return callback(std::move(out)); } else { status = 500; return false; @@ -2611,7 +2615,7 @@ bool prepare_content_receiver(T &x, int &status, ContentReceiver receiver, ContentReceiver out = [&](const char *buf, size_t n) { return receiver(buf, n); }; - return callback(out); + return callback(std::move(out)); } template @@ -2619,7 +2623,7 @@ bool read_content(Stream &strm, T &x, size_t payload_max_length, int &status, Progress progress, ContentReceiver receiver, bool decompress) { return prepare_content_receiver( - x, status, receiver, decompress, [&](const ContentReceiver &out) { + x, status, std::move(receiver), decompress, [&](const ContentReceiver &out) { auto ret = true; auto exceed_payload_max_length = false; @@ -2634,7 +2638,7 @@ bool read_content(Stream &strm, T &x, size_t payload_max_length, int &status, skip_content_with_length(strm, len); ret = false; } else if (len > 0) { - ret = read_content_with_length(strm, len, progress, out); + ret = read_content_with_length(strm, len, std::move(progress), out); } } @@ -3451,7 +3455,7 @@ inline std::pair make_range_header(Ranges ranges) { if (r.second != -1) { field += std::to_string(r.second); } i++; } - return std::make_pair("Range", field); + return std::make_pair("Range", std::move(field)); } inline std::pair @@ -3460,7 +3464,7 @@ make_basic_authentication_header(const std::string &username, bool is_proxy = false) { auto field = "Basic " + detail::base64_encode(username + ":" + password); auto key = is_proxy ? "Proxy-Authorization" : "Authorization"; - return std::make_pair(key, field); + return std::make_pair(key, std::move(field)); } inline std::pair @@ -3468,7 +3472,7 @@ make_bearer_token_authentication_header(const std::string &token, bool is_proxy = false) { auto field = "Bearer " + token; auto key = is_proxy ? "Proxy-Authorization" : "Authorization"; - return std::make_pair(key, field); + return std::make_pair(key, std::move(field)); } // Request implementation @@ -3761,60 +3765,66 @@ inline Server::Server() inline Server::~Server() {} inline Server &Server::Get(const char *pattern, Handler handler) { - get_handlers_.push_back(std::make_pair(std::regex(pattern), handler)); + get_handlers_.push_back(std::make_pair(std::regex(pattern), + std::move(handler))); return *this; } inline Server &Server::Post(const char *pattern, Handler handler) { - post_handlers_.push_back(std::make_pair(std::regex(pattern), handler)); + post_handlers_.push_back(std::make_pair(std::regex(pattern), + std::move(handler))); return *this; } inline Server &Server::Post(const char *pattern, HandlerWithContentReader handler) { post_handlers_for_content_reader_.push_back( - std::make_pair(std::regex(pattern), handler)); + std::make_pair(std::regex(pattern), std::move(handler))); return *this; } inline Server &Server::Put(const char *pattern, Handler handler) { - put_handlers_.push_back(std::make_pair(std::regex(pattern), handler)); + put_handlers_.push_back(std::make_pair(std::regex(pattern), + std::move(handler))); return *this; } inline Server &Server::Put(const char *pattern, HandlerWithContentReader handler) { put_handlers_for_content_reader_.push_back( - std::make_pair(std::regex(pattern), handler)); + std::make_pair(std::regex(pattern), std::move(handler))); return *this; } inline Server &Server::Patch(const char *pattern, Handler handler) { - patch_handlers_.push_back(std::make_pair(std::regex(pattern), handler)); + patch_handlers_.push_back(std::make_pair(std::regex(pattern), + std::move(handler))); return *this; } inline Server &Server::Patch(const char *pattern, HandlerWithContentReader handler) { patch_handlers_for_content_reader_.push_back( - std::make_pair(std::regex(pattern), handler)); + std::make_pair(std::regex(pattern), std::move(handler))); return *this; } inline Server &Server::Delete(const char *pattern, Handler handler) { - delete_handlers_.push_back(std::make_pair(std::regex(pattern), handler)); + delete_handlers_.push_back(std::make_pair(std::regex(pattern), + std::move(handler))); return *this; } inline Server &Server::Delete(const char *pattern, HandlerWithContentReader handler) { delete_handlers_for_content_reader_.push_back( - std::make_pair(std::regex(pattern), handler)); + std::make_pair(std::regex(pattern), std::move(handler))); return *this; } inline Server &Server::Options(const char *pattern, Handler handler) { - options_handlers_.push_back(std::make_pair(std::regex(pattern), handler)); + options_handlers_.push_back(std::make_pair(std::regex(pattern), + std::move(handler))); return *this; } @@ -3860,7 +3870,7 @@ inline void Server::set_error_handler(Handler handler) { inline void Server::set_tcp_nodelay(bool on) { tcp_nodelay_ = on; } inline void Server::set_socket_options(SocketOptions socket_options) { - socket_options_ = socket_options; + socket_options_ = std::move(socket_options); } inline void Server::set_logger(Logger logger) { logger_ = std::move(logger); } @@ -4198,8 +4208,9 @@ inline bool Server::read_content_with_content_receiver( Stream &strm, Request &req, Response &res, ContentReceiver receiver, MultipartContentHeader multipart_header, ContentReceiver multipart_receiver) { - return read_content_core(strm, req, res, receiver, multipart_header, - multipart_receiver); + return read_content_core(strm, req, res, std::move(receiver), + std::move(multipart_header), + std::move(multipart_receiver)); } inline bool Server::read_content_core(Stream &strm, Request &req, Response &res, @@ -4230,11 +4241,11 @@ inline bool Server::read_content_core(Stream &strm, Request &req, Response &res, } return true; */ - return multipart_form_data_parser.parse(buf, n, multipart_receiver, - mulitpart_header); + return multipart_form_data_parser.parse(buf, n, std::move(multipart_receiver), + std::move(mulitpart_header)); }; } else { - out = receiver; + out = std::move(receiver); } if (req.method == "DELETE" && !req.has_header("Content-Length")) { @@ -4290,7 +4301,7 @@ inline socket_t Server::create_server_socket(const char *host, int port, int socket_flags, SocketOptions socket_options) const { return detail::create_socket( - host, port, socket_flags, tcp_nodelay_, socket_options, + host, port, socket_flags, tcp_nodelay_, std::move(socket_options), [](socket_t sock, struct addrinfo &ai) -> bool { if (::bind(sock, ai.ai_addr, static_cast(ai.ai_addrlen))) { return false; @@ -4392,32 +4403,38 @@ inline bool Server::routing(Request &req, Response &res, Stream &strm) { { ContentReader reader( [&](ContentReceiver receiver) { - return read_content_with_content_receiver(strm, req, res, receiver, + return read_content_with_content_receiver(strm, req, res, + std::move(receiver), nullptr, nullptr); }, [&](MultipartContentHeader header, ContentReceiver receiver) { return read_content_with_content_receiver(strm, req, res, nullptr, - header, receiver); + std::move(header), + std::move(receiver)); }); if (req.method == "POST") { if (dispatch_request_for_content_reader( - req, res, reader, post_handlers_for_content_reader_)) { + req, res, std::move(reader), + post_handlers_for_content_reader_)) { return true; } } else if (req.method == "PUT") { if (dispatch_request_for_content_reader( - req, res, reader, put_handlers_for_content_reader_)) { + req, res, std::move(reader), + put_handlers_for_content_reader_)) { return true; } } else if (req.method == "PATCH") { if (dispatch_request_for_content_reader( - req, res, reader, patch_handlers_for_content_reader_)) { + req, res, std::move(reader), + patch_handlers_for_content_reader_)) { return true; } } else if (req.method == "DELETE") { if (dispatch_request_for_content_reader( - req, res, reader, delete_handlers_for_content_reader_)) { + req, res, std::move(reader), + delete_handlers_for_content_reader_)) { return true; } } @@ -4477,7 +4494,7 @@ inline bool Server::dispatch_request_for_content_reader( const auto &handler = x.second; if (std::regex_match(req.path, req.matches, pattern)) { - handler(req, res, content_reader); + handler(req, res, std::move(content_reader)); return true; } } @@ -5036,7 +5053,7 @@ inline std::shared_ptr ClientImpl::send_with_content_provider( { if (content_provider) { req.content_length = content_length; - req.content_provider = content_provider; + req.content_provider = std::move(content_provider); } else { req.body = body; } @@ -5090,7 +5107,8 @@ inline bool ClientImpl::process_request(Stream &strm, const Request &req, int dummy_status; if (!detail::read_content(strm, res, (std::numeric_limits::max)(), - dummy_status, progress, out, decompress_)) { + dummy_status, std::move(progress), std::move(out), + decompress_)) { if (error_ != Error::Canceled) { error_ = Error::Read; } return false; } @@ -5112,7 +5130,8 @@ ClientImpl::process_socket(Socket &socket, std::function callback) { return detail::process_client_socket(socket.sock, read_timeout_sec_, read_timeout_usec_, write_timeout_sec_, - write_timeout_usec_, callback); + write_timeout_usec_, + std::move(callback)); } inline bool ClientImpl::is_ssl() const { return false; } @@ -5170,23 +5189,23 @@ inline Result ClientImpl::Get(const char *path, const Headers &headers, inline Result ClientImpl::Get(const char *path, ResponseHandler response_handler, ContentReceiver content_receiver) { - return Get(path, Headers(), std::move(response_handler), content_receiver, - nullptr); + return Get(path, Headers(), std::move(response_handler), + std::move(content_receiver), nullptr); } inline Result ClientImpl::Get(const char *path, const Headers &headers, ResponseHandler response_handler, ContentReceiver content_receiver) { - return Get(path, headers, std::move(response_handler), content_receiver, - nullptr); + return Get(path, headers, std::move(response_handler), + std::move(content_receiver), nullptr); } inline Result ClientImpl::Get(const char *path, ResponseHandler response_handler, ContentReceiver content_receiver, Progress progress) { - return Get(path, Headers(), std::move(response_handler), content_receiver, - progress); + return Get(path, Headers(), std::move(response_handler), + std::move(content_receiver), std::move(progress)); } inline Result ClientImpl::Get(const char *path, const Headers &headers, @@ -5247,7 +5266,8 @@ inline Result ClientImpl::Post(const char *path, const Params ¶ms) { inline Result ClientImpl::Post(const char *path, size_t content_length, ContentProvider content_provider, const char *content_type) { - return Post(path, Headers(), content_length, content_provider, content_type); + return Post(path, Headers(), content_length, std::move(content_provider), + content_type); } inline Result ClientImpl::Post(const char *path, const Headers &headers, @@ -5255,7 +5275,8 @@ inline Result ClientImpl::Post(const char *path, const Headers &headers, ContentProvider content_provider, const char *content_type) { auto ret = send_with_content_provider("POST", path, headers, std::string(), - content_length, content_provider, + content_length, + std::move(content_provider), content_type); return Result{ret, get_last_error()}; } @@ -5317,7 +5338,8 @@ inline Result ClientImpl::Put(const char *path, const Headers &headers, inline Result ClientImpl::Put(const char *path, size_t content_length, ContentProvider content_provider, const char *content_type) { - return Put(path, Headers(), content_length, content_provider, content_type); + return Put(path, Headers(), content_length, std::move(content_provider), + content_type); } inline Result ClientImpl::Put(const char *path, const Headers &headers, @@ -5325,7 +5347,8 @@ inline Result ClientImpl::Put(const char *path, const Headers &headers, ContentProvider content_provider, const char *content_type) { auto ret = send_with_content_provider("PUT", path, headers, std::string(), - content_length, content_provider, + content_length, + std::move(content_provider), content_type); return Result{ret, get_last_error()}; } @@ -5356,7 +5379,8 @@ inline Result ClientImpl::Patch(const char *path, const Headers &headers, inline Result ClientImpl::Patch(const char *path, size_t content_length, ContentProvider content_provider, const char *content_type) { - return Patch(path, Headers(), content_length, content_provider, content_type); + return Patch(path, Headers(), content_length, std::move(content_provider), + content_type); } inline Result ClientImpl::Patch(const char *path, const Headers &headers, @@ -5364,7 +5388,8 @@ inline Result ClientImpl::Patch(const char *path, const Headers &headers, ContentProvider content_provider, const char *content_type) { auto ret = send_with_content_provider("PATCH", path, headers, std::string(), - content_length, content_provider, + content_length, + std::move(content_provider), content_type); return Result{ret, get_last_error()}; } @@ -5479,7 +5504,7 @@ inline void ClientImpl::set_default_headers(Headers headers) { inline void ClientImpl::set_tcp_nodelay(bool on) { tcp_nodelay_ = on; } inline void ClientImpl::set_socket_options(SocketOptions socket_options) { - socket_options_ = socket_options; + socket_options_ = std::move(socket_options); } inline void ClientImpl::set_compress(bool on) { compress_ = on; } @@ -6024,7 +6049,7 @@ SSLClient::process_socket(Socket &socket, assert(socket.ssl); return detail::process_client_socket_ssl( socket.ssl, socket.sock, read_timeout_sec_, read_timeout_usec_, - write_timeout_sec_, write_timeout_usec_, callback); + write_timeout_sec_, write_timeout_usec_, std::move(callback)); } inline bool SSLClient::is_ssl() const { return true; } @@ -6221,11 +6246,11 @@ inline Result Client::Get(const char *path, const Headers &headers) { return cli_->Get(path, headers); } inline Result Client::Get(const char *path, Progress progress) { - return cli_->Get(path, progress); + return cli_->Get(path, std::move(progress)); } inline Result Client::Get(const char *path, const Headers &headers, Progress progress) { - return cli_->Get(path, headers, progress); + return cli_->Get(path, headers, std::move(progress)); } inline Result Client::Get(const char *path, ContentReceiver content_receiver) { return cli_->Get(path, std::move(content_receiver)); @@ -6262,7 +6287,8 @@ inline Result Client::Get(const char *path, ResponseHandler response_handler, inline Result Client::Get(const char *path, const Headers &headers, ResponseHandler response_handler, ContentReceiver content_receiver, Progress progress) { - return cli_->Get(path, headers, response_handler, content_receiver, progress); + return cli_->Get(path, headers, std::move(response_handler), + std::move(content_receiver), std::move(progress)); } inline Result Client::Head(const char *path) { return cli_->Head(path); } @@ -6282,13 +6308,14 @@ inline Result Client::Post(const char *path, const Headers &headers, inline Result Client::Post(const char *path, size_t content_length, ContentProvider content_provider, const char *content_type) { - return cli_->Post(path, content_length, content_provider, content_type); + return cli_->Post(path, content_length, std::move(content_provider), + content_type); } inline Result Client::Post(const char *path, const Headers &headers, size_t content_length, ContentProvider content_provider, const char *content_type) { - return cli_->Post(path, headers, content_length, content_provider, + return cli_->Post(path, headers, content_length, std::move(content_provider), content_type); } inline Result Client::Post(const char *path, const Params ¶ms) { @@ -6318,13 +6345,14 @@ inline Result Client::Put(const char *path, const Headers &headers, inline Result Client::Put(const char *path, size_t content_length, ContentProvider content_provider, const char *content_type) { - return cli_->Put(path, content_length, content_provider, content_type); + return cli_->Put(path, content_length, std::move(content_provider), + content_type); } inline Result Client::Put(const char *path, const Headers &headers, size_t content_length, ContentProvider content_provider, const char *content_type) { - return cli_->Put(path, headers, content_length, content_provider, + return cli_->Put(path, headers, content_length, std::move(content_provider), content_type); } inline Result Client::Put(const char *path, const Params ¶ms) { @@ -6345,13 +6373,14 @@ inline Result Client::Patch(const char *path, const Headers &headers, inline Result Client::Patch(const char *path, size_t content_length, ContentProvider content_provider, const char *content_type) { - return cli_->Patch(path, content_length, content_provider, content_type); + return cli_->Patch(path, content_length, std::move(content_provider), + content_type); } inline Result Client::Patch(const char *path, const Headers &headers, size_t content_length, ContentProvider content_provider, const char *content_type) { - return cli_->Patch(path, headers, content_length, content_provider, + return cli_->Patch(path, headers, content_length, std::move(content_provider), content_type); } inline Result Client::Delete(const char *path) { return cli_->Delete(path); } @@ -6386,7 +6415,7 @@ inline void Client::set_default_headers(Headers headers) { inline void Client::set_tcp_nodelay(bool on) { cli_->set_tcp_nodelay(on); } inline void Client::set_socket_options(SocketOptions socket_options) { - cli_->set_socket_options(socket_options); + cli_->set_socket_options(std::move(socket_options)); } inline void Client::set_connection_timeout(time_t sec, time_t usec) { From ff74092eae85f9fc1cd0b50dafcb99a63fa902cc Mon Sep 17 00:00:00 2001 From: Andrew Gasparovic <72571446+agasparovic@users.noreply.github.com> Date: Sun, 11 Oct 2020 15:13:46 -0700 Subject: [PATCH 2/2] Fix two use-after-move errors --- httplib.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/httplib.h b/httplib.h index 2039f5b198..fa2b993301 100644 --- a/httplib.h +++ b/httplib.h @@ -2547,8 +2547,7 @@ inline bool read_content_chunked(Stream &strm, ContentReceiver out) { if (chunk_len == 0) { break; } - if (!read_content_with_length(strm, chunk_len, nullptr, - std::move(out))) { + if (!read_content_with_length(strm, chunk_len, nullptr, out)) { return false; } @@ -4494,7 +4493,7 @@ inline bool Server::dispatch_request_for_content_reader( const auto &handler = x.second; if (std::regex_match(req.path, req.matches, pattern)) { - handler(req, res, std::move(content_reader)); + handler(req, res, content_reader); return true; } }