Fixed string_view c++17 compatibility

string_view isn't supposed to be implicitly convertible to std::string
and code would break compiling under c++17 (when our local string_view
is simply a std::string_view typedef).
This commit is contained in:
Jason Rhinelander 2020-02-24 22:20:56 -04:00
parent 3c1a0c4280
commit e4d371b026
9 changed files with 67 additions and 49 deletions

View File

@ -145,7 +145,7 @@ bt_list_consumer::bt_list_consumer(string_view data_) : data{std::move(data_)} {
/// Attempt to parse the next value as a string (and advance just past it). Throws if the next
/// value is not a string.
string_view bt_list_consumer::consume_string() {
string_view bt_list_consumer::consume_string_view() {
if (data.empty())
throw bt_deserialize_invalid{"expected a string, but reached end of data"};
else if (!is_string())
@ -156,10 +156,14 @@ string_view bt_list_consumer::consume_string() {
return result;
}
std::string bt_list_consumer::consume_string() {
return std::string{consume_string_view()};
}
/// Consumes a value without returning it.
void bt_list_consumer::skip_value() {
if (is_string())
consume_string();
consume_string_view();
else if (is_integer())
detail::bt_deserialize_integer(data);
else if (is_list())
@ -188,7 +192,7 @@ string_view bt_list_consumer::consume_dict_data() {
if (data.size() < 2 || !is_dict()) throw bt_deserialize_invalid_type{"next bt value is not a dict"};
data.remove_prefix(1); // Descent into the dict, consumer the "d"
while (!is_finished()) {
consume_string(); // Key is always a string
consume_string_view(); // Key is always a string
if (!data.empty())
skip_value();
if (data.empty())
@ -210,7 +214,7 @@ bool bt_dict_consumer::consume_key() {
return true;
if (data.empty()) throw bt_deserialize_invalid_type{"expected a key or dict end, found end of string"};
if (data[0] == 'e') return false;
key_ = bt_list_consumer::consume_string();
key_ = bt_list_consumer::consume_string_view();
if (data.empty() || data[0] == 'e')
throw bt_deserialize_invalid{"dict key isn't followed by a value"};
return true;
@ -220,7 +224,7 @@ std::pair<string_view, string_view> bt_dict_consumer::next_string() {
if (!is_string())
throw bt_deserialize_invalid_type{"expected a string, but found "s + data.front()};
std::pair<string_view, string_view> ret;
ret.second = bt_list_consumer::consume_string();
ret.second = bt_list_consumer::consume_string_view();
ret.first = flush_key();
return ret;
}

View File

@ -86,6 +86,7 @@ class bt_dict;
/// Recursive generic type that can fully represent everything valid for a BT serialization.
using bt_value = mapbox::util::variant<
std::string,
string_view,
int64_t,
mapbox::util::recursive_wrapper<bt_list>,
mapbox::util::recursive_wrapper<bt_dict>
@ -572,7 +573,8 @@ public:
/// Attempt to parse the next value as a string (and advance just past it). Throws if the next
/// value is not a string.
string_view consume_string();
std::string consume_string();
string_view consume_string_view();
/// Attempts to parse the next value as an integer (and advance just past it). Throws if the
/// next value is not an integer.
@ -798,7 +800,8 @@ public:
/// value = d.consume_string();
///
auto consume_string() { return next_string().second; }
auto consume_string_view() { return next_string().second; }
auto consume_string() { return std::string{consume_string_view()}; }
template <typename IntType>
auto consume_integer() { return next_integer<IntType>().second; }

View File

@ -119,13 +119,17 @@ std::vector<std::string> as_strings(const MessageContainer& msgs) {
return result;
}
// Returns a string view of the given message data. If real std::string_views are available,
// returns one, otherwise returns a simple partial implementation of string_view. It's the caller's
// responsibility to keep the referenced message alive.
string_view view(const zmq::message_t &m) {
// Returns a string view of the given message data. It's the caller's responsibility to keep the
// referenced message alive.
string_view view(const zmq::message_t& m) {
return {m.data<char>(), m.size()};
}
// Like the above, but copies the data into a string.
std::string string(const zmq::message_t& m) {
return std::string{view(m)};
}
// Builds a ZMTP metadata key-value pair. These will be available on every message from that peer.
// Keys must start with X- and be <= 255 characters.
std::string zmtp_metadata(string_view key, string_view value) {
@ -575,7 +579,8 @@ void LokiMQ::setup_outgoing_socket(zmq::socket_t& socket, string_view remote_pub
}
std::pair<zmq::socket_t *, std::string>
LokiMQ::proxy_connect_sn(const std::string &remote, const std::string &connect_hint, bool optional, bool incoming_only, std::chrono::milliseconds keep_alive) {
LokiMQ::proxy_connect_sn(string_view remote_sv, string_view connect_hint, bool optional, bool incoming_only, std::chrono::milliseconds keep_alive) {
std::string remote{remote_sv};
auto &peer = peers[remote]; // We may auto-vivify here, but that's okay; it'll get cleaned up in idle_expiry if no connection gets established
std::pair<zmq::socket_t *, std::string> result = {nullptr, ""s};
@ -614,7 +619,7 @@ LokiMQ::proxy_connect_sn(const std::string &remote, const std::string &connect_h
// special inproc connection if self that doesn't need any external connection
addr = SN_ADDR_SELF;
} else {
addr = connect_hint;
addr = std::string{connect_hint};
if (addr.empty())
addr = peer_lookup(remote);
else
@ -657,7 +662,7 @@ std::pair<zmq::socket_t *, std::string> LokiMQ::proxy_connect_sn(bt_dict &&data)
void LokiMQ::proxy_send(bt_dict_consumer data) {
// NB: bt_dict_consumer goes in alphabetical order
std::string hint;
string_view hint;
std::chrono::milliseconds keep_alive{DEFAULT_SEND_KEEP_ALIVE};
bool optional = false;
bool incoming = false;
@ -665,7 +670,7 @@ void LokiMQ::proxy_send(bt_dict_consumer data) {
std::string request_tag;
std::unique_ptr<ReplyCallback> request_cbptr;
if (data.skip_until("hint"))
hint = data.consume_string();
hint = data.consume_string_view();
if (data.skip_until("incoming"))
incoming = data.consume_integer<bool>();
if (data.skip_until("keep-alive"))
@ -674,7 +679,7 @@ void LokiMQ::proxy_send(bt_dict_consumer data) {
optional = data.consume_integer<bool>();
if (!data.skip_until("pubkey"))
throw std::runtime_error("Internal error: Invalid proxy send command; pubkey missing");
std::string remote_pubkey = data.consume_string();
string_view remote_pubkey = data.consume_string_view();
if (data.skip_until("request"))
request = data.consume_integer<bool>();
if (request) {
@ -710,7 +715,7 @@ void LokiMQ::proxy_send(bt_dict_consumer data) {
if (e.num() == EHOSTUNREACH && sock_route.first == &listener && !sock_route.second.empty()) {
// We *tried* to route via the incoming connection but it is no longer valid. Drop it,
// establish a new connection, and try again.
auto &peer = peers[remote_pubkey];
auto &peer = peers[std::string{remote_pubkey}];
peer.incoming.clear(); // Don't worry about cleaning the map entry if outgoing is also < 0: that will happen at the next idle cleanup
LMQ_LOG(debug, "Could not route back to SN ", to_hex(remote_pubkey), " via listening socket; trying via new outgoing connection");
return proxy_send(std::move(data));
@ -949,7 +954,7 @@ void LokiMQ::proxy_loop() {
#ifndef NDEBUG
if (log_level() >= LogLevel::trace) {
LMQ_TRACE("Reserving space for ", max_workers, " max workers = ", general_workers, " general + reserved:");
LMQ_TRACE("Reserving space for ", max_workers, " max workers = ", general_workers, " general plus reservations for:");
for (const auto& cat : categories)
LMQ_TRACE(" - ", cat.first, ": ", cat.second.reserved_threads);
LMQ_TRACE(" - (batch jobs): ", batch_jobs_reserved);
@ -1106,7 +1111,7 @@ std::pair<LokiMQ::category*, const std::pair<LokiMQ::CommandCallback, bool>*> Lo
LMQ_LOG(warn, "Invalid command '", command, "': expected <category>.<command>");
return {};
}
string_view catname{&command[0], dot};
std::string catname = command.substr(0, dot);
std::string cmd = command.substr(dot + 1);
auto catit = categories.find(catname);
@ -1243,7 +1248,7 @@ bool LokiMQ::proxy_handle_builtin(int conn_index, std::vector<zmq::message_t>& p
LMQ_LOG(warn, "Received REPLY without a reply tag; ignoring");
return true;
}
std::string reply_tag = view(parts[tag_pos]);
std::string reply_tag{view(parts[tag_pos])};
auto it = pending_requests.find(reply_tag);
if (it != pending_requests.end()) {
LMQ_LOG(debug, "Received REPLY for pending command", to_hex(reply_tag), "; scheduling callback");
@ -1265,7 +1270,7 @@ bool LokiMQ::proxy_handle_builtin(int conn_index, std::vector<zmq::message_t>& p
return true;
}
LMQ_LOG(info, "Incoming client from ", peer_address(parts.back()), " sent HI, replying with HELLO");
send_routed_message(listener, route, "HELLO");
send_routed_message(listener, std::string{route}, "HELLO");
return true;
} else if (cmd == "HELLO") {
if (!is_outgoing_conn) {
@ -1394,7 +1399,7 @@ void LokiMQ::proxy_to_worker(size_t conn_index, std::vector<zmq::message_t>& par
if (is_outgoing_conn)
send_direct_message(remotes[conn_index - listener.connected()].second, "UNKNOWNCOMMAND", command);
else
send_routed_message(listener, pubkey, "UNKNOWNCOMMAND", command);
send_routed_message(listener, std::string{pubkey}, "UNKNOWNCOMMAND", command);
return;
}
@ -1419,7 +1424,7 @@ void LokiMQ::proxy_to_worker(size_t conn_index, std::vector<zmq::message_t>& par
}
LMQ_LOG(debug, "No available free workers, queuing ", command, " for later");
pending_commands.emplace_back(category, std::move(command), std::move(data_parts), cat_call.second, pubkey, peer_info.service_node);
pending_commands.emplace_back(category, std::move(command), std::move(data_parts), cat_call.second, std::string{pubkey}, peer_info.service_node);
category.queued++;
return;
}
@ -1433,7 +1438,7 @@ void LokiMQ::proxy_to_worker(size_t conn_index, std::vector<zmq::message_t>& par
run.is_batch_job = false;
run.cat = &category;
run.command = std::move(command);
run.pubkey = pubkey;
run.pubkey = std::string{pubkey};
run.service_node = peer_info.service_node;
run.data_parts = std::move(data_parts);
run.callback = cat_call.second;
@ -1445,7 +1450,7 @@ void LokiMQ::proxy_to_worker(size_t conn_index, std::vector<zmq::message_t>& par
// it has changed (e.g. new connection).
auto route = view(parts[0]);
if (string_view(peer_info.incoming) != route)
peer_info.incoming = route;
peer_info.incoming = std::string{route};
}
LMQ_TRACE("Forwarding incoming ", run.command, " from ", run.service_node ? "SN " : "non-SN ",
@ -1475,9 +1480,9 @@ bool LokiMQ::proxy_check_auth(string_view pubkey, size_t conn_index, const peer_
// issuing. Drop the connection; if the remote has something important to relay it will
// reconnect, at which point we will reassess the SN status on the new incoming connection.
if (!is_outgoing_conn)
send_routed_message(listener, pubkey, "BYE");
send_routed_message(listener, std::string{pubkey}, "BYE");
else
proxy_disconnect(pubkey);
proxy_disconnect(std::string{pubkey});
return false;
}
@ -1487,7 +1492,7 @@ bool LokiMQ::proxy_check_auth(string_view pubkey, size_t conn_index, const peer_
if (is_outgoing_conn)
send_direct_message(remotes[conn_index - listener.connected()].second, std::move(reply), command);
else
send_routed_message(listener, pubkey, std::move(reply), command);
send_routed_message(listener, std::string{pubkey}, std::move(reply), command);
return true;
}
@ -1537,7 +1542,7 @@ void LokiMQ::process_zap_requests(zmq::socket_t &zap_auth) {
std::vector<std::string> response_vals(6);
response_vals[0] = "1.0"; // version
if (frames.size() >= 2)
response_vals[1] = view(frames[1]); // unique identifier
response_vals[1] = std::string{view(frames[1])}; // unique identifier
std::string &status_code = response_vals[2], &status_text = response_vals[3];
if (frames.size() < 6 || view(frames[0]) != "1.0") {
@ -1562,7 +1567,7 @@ void LokiMQ::process_zap_requests(zmq::socket_t &zap_auth) {
status_code = "400";
status_text = "Unknown authentication domain: " + std::string{domain};
} else {
auto ip = view(frames[3]), pubkey = view(frames[6]);
auto ip = string(frames[3]), pubkey = string(frames[6]);
auto result = allow_connection(ip, pubkey);
bool sn = result.remote_sn;
auto& user_id = response_vals[4];
@ -1649,8 +1654,9 @@ void LokiMQ::proxy_connect_remote(bt_dict_consumer data) {
remote_pubkey = data.consume_string();
assert(remote_pubkey.size() == 32 || remote_pubkey.empty());
}
if (data.skip_until("remote"))
if (data.skip_until("remote")) {
remote = data.consume_string();
}
if (data.skip_until("timeout"))
timeout = std::chrono::milliseconds{data.consume_integer<uint64_t>()};

View File

@ -115,7 +115,7 @@ public:
/// If you want to send a non-strong reply even when the remote is a service node then add
/// an explicit `send_option::optional()` argument.
template <typename... Args>
void send_back(const std::string& command, Args&&... args);
void send_back(string_view, Args&&... args);
/// Sends a reply to a request. This takes no command: the command is always the built-in
/// "REPLY" command, followed by the unique reply tag, then any reply data parts. All other
@ -204,12 +204,12 @@ public:
///
/// @returns an `AuthLevel` enum value indicating the default auth level for the incoming
/// connection, or AuthLevel::denied if the connection should be refused.
using AllowFunc = std::function<Allow(string_view ip, string_view pubkey)>;
using AllowFunc = std::function<Allow(const std::string &ip, const std::string &pubkey)>;
/// Callback that is invoked when we need to send a "strong" message to a SN that we aren't
/// already connected to and need to establish a connection. This callback returns the ZMQ
/// connection string we should use which is typically a string such as `tcp://1.2.3.4:5678`.
using SNRemoteAddress = std::function<std::string(const std::string& pubkey)>;
using SNRemoteAddress = std::function<std::string(const std::string &pubkey)>;
/// The callback type for registered commands.
using CommandCallback = std::function<void(Message& message)>;
@ -440,7 +440,7 @@ private:
/// Common connection implementation used by proxy_connect/proxy_send. Returns the socket
/// and, if a routing prefix is needed, the required prefix (or an empty string if not needed).
/// For an optional connect that fail, returns nullptr for the socket.
std::pair<zmq::socket_t*, std::string> proxy_connect_sn(const std::string& pubkey, const std::string& connect_hint, bool optional, bool incoming_only, std::chrono::milliseconds keep_alive);
std::pair<zmq::socket_t*, std::string> proxy_connect_sn(string_view pubkey, string_view connect_hint, bool optional, bool incoming_only, std::chrono::milliseconds keep_alive);
/// CONNECT_SN command telling us to connect to a new pubkey. Returns the socket (which could be
/// existing or a new one).
@ -848,7 +848,7 @@ public:
* performing a connection address lookup on the pubkey.
*/
template <typename... T>
void send(const std::string& pubkey, const std::string& cmd, const T&... opts);
void send(string_view pubkey, string_view cmd, const T&... opts);
/** Send a command configured as a "REQUEST" command: the data parts will be prefixed with a
* random identifier. The remote is expected to reply with a ["REPLY", <identifier>, ...]
@ -862,8 +862,7 @@ public:
* @param opts - anything else (i.e. strings, send_options) is forwarded to send().
*/
template <typename... T>
void request(const std::string& pubkey, const std::string& cmd, ReplyCallback callback,
const T&... opts);
void request(string_view pubkey, string_view cmd, ReplyCallback callback, const T&... opts);
/// The key pair this LokiMQ was created with; if empty keys were given during construction then
/// this returns the generated keys.
@ -942,7 +941,7 @@ void send_control(zmq::socket_t& sock, string_view cmd, std::string data = {});
/// Base case: takes a string-like value and appends it to the message parts
inline void apply_send_option(bt_list& parts, bt_dict&, string_view arg) {
parts.push_back(arg);
parts.emplace_back(arg);
}
/// `data_parts` specialization: appends a range of serialized data parts to the parts to send
@ -975,7 +974,7 @@ inline void apply_send_option(bt_list&, bt_dict& control_data, const send_option
} // namespace detail
template <typename... T>
void LokiMQ::send(const std::string& pubkey, const std::string& cmd, const T &...opts) {
void LokiMQ::send(string_view pubkey, string_view cmd, const T &...opts) {
bt_dict control_data;
bt_list parts{{cmd}};
#ifdef __cpp_fold_expressions
@ -992,7 +991,7 @@ void LokiMQ::send(const std::string& pubkey, const std::string& cmd, const T &..
std::string make_random_string(size_t size);
template <typename... T>
void LokiMQ::request(const std::string& pubkey, const std::string& cmd, ReplyCallback callback, const T &...opts) {
void LokiMQ::request(string_view pubkey, string_view cmd, ReplyCallback callback, const T &...opts) {
auto reply_tag = make_random_string(15); // 15 should keep us in most stl implementations' small string optimization
bt_dict control_data;
bt_list parts{{cmd, reply_tag}};
@ -1011,7 +1010,7 @@ void LokiMQ::request(const std::string& pubkey, const std::string& cmd, ReplyCal
}
template <typename... Args>
void Message::send_back(const std::string& command, Args&&... args) {
void Message::send_back(string_view command, Args&&... args) {
assert(reply_tag.empty());
if (service_node) lokimq.send(pubkey, command, std::forward<Args>(args)...);
else lokimq.send(pubkey, command, send_option::optional{}, std::forward<Args>(args)...);
@ -1034,7 +1033,7 @@ void LokiMQ::log_(LogLevel lvl, const char* file, int line, const T&... stuff) {
std::ostringstream os;
#ifdef __cpp_fold_expressions
os << ... << stuff;
(os << ... << stuff);
#else
(void) std::initializer_list<int>{(os << stuff, 0)...};
#endif

View File

@ -73,7 +73,7 @@ public:
constexpr size_t length() const noexcept { return size_; }
constexpr size_t max_size() const noexcept { return std::numeric_limits<size_t>::max(); }
constexpr bool empty() const noexcept { return size_ == 0; }
operator std::string() const { return {data_, size_}; }
explicit operator std::string() const { return {data_, size_}; }
constexpr const char* begin() const noexcept { return data_; }
constexpr const char* cbegin() const noexcept { return data_; }
constexpr const char* end() const noexcept { return data_ + size_; }

View File

@ -45,7 +45,7 @@ TEST_CASE("batching many small jobs", "[batch-many]") {
"", "", // generate ephemeral keys
false, // not a service node
{}, // don't listen
[](auto &) { return ""; },
[](auto) { return ""; },
[](auto ip, auto pk) { return lokimq::Allow{lokimq::AuthLevel::none, false}; },
};
lmq.set_general_threads(4);
@ -63,7 +63,7 @@ TEST_CASE("batch exception propagation", "[batch-exceptions]") {
"", "", // generate ephemeral keys
false, // not a service node
{}, // don't listen
[](auto &) { return ""; },
[](auto) { return ""; },
[](auto ip, auto pk) { return lokimq::Allow{lokimq::AuthLevel::none, false}; },
};
lmq.set_general_threads(4);

View File

@ -25,7 +25,7 @@ TEST_CASE("basic commands", "[commands]") {
});
std::string client_pubkey;
server.add_command("public", "client.pubkey", [&](Message& m) {
client_pubkey = m.pubkey;
client_pubkey = std::string{m.pubkey};
});
server.start();

View File

@ -11,11 +11,11 @@ TEST_CASE("connections", "[connect][curve]") {
"", "", // generate ephemeral keys
false, // not a service node
{listen},
[](auto &) { return ""; },
[](auto) { return ""; },
[](auto /*ip*/, auto /*pk*/) { return Allow{AuthLevel::none, false}; },
get_logger("")
};
// server.log_level(LogLevel::trace);
server.log_level(LogLevel::trace);
server.add_category("public", Access{AuthLevel::none});
server.add_request_command("public", "hello", [&](Message& m) { m.send_reply("hi"); });

View File

@ -170,8 +170,14 @@ TEST_CASE("string view", "[string_view]") {
REQUIRE( string_view{"abc"} != string_view{"abcd"} );
REQUIRE( string_view{"abc"} < string_view{"abcd"} );
REQUIRE( string_view{"abc"} < string_view{"abd"} );
REQUIRE_FALSE( string_view{"abd"} < string_view{"abc"} );
REQUIRE_FALSE( string_view{"abcd"} < string_view{"abc"} );
REQUIRE_FALSE( string_view{"abc"} < string_view{"abc"} );
REQUIRE( string_view{"abd"} > string_view{"abc"} );
REQUIRE( string_view{"abcd"} > string_view{"abc"} );
REQUIRE_FALSE( string_view{"abc"} > string_view{"abd"} );
REQUIRE_FALSE( string_view{"abc"} > string_view{"abcd"} );
REQUIRE_FALSE( string_view{"abc"} > string_view{"abc"} );
REQUIRE( string_view{"abc"} <= string_view{"abcd"} );
REQUIRE( string_view{"abc"} <= string_view{"abc"} );
REQUIRE( string_view{"abc"} <= string_view{"abd"} );