Improve naming and make timeouts unsigned (#43)

When working on Java bindings, I noticed that some names were
not completely obvious. At the same time, I also noticed it was
possible to have negative timeouts, which I don't want.

Author: bassosimone

libndt-client.cpp

@@ -33,7 +33,7 @@ int main(int, char **argv) {
   using namespace measurement_kit;
   libndt::Settings settings;
   settings.verbosity = libndt::verbosity::quiet;
-  settings.test_suite = 0;  // you need to enable tests explicitly
+  settings.nettest_flags = 0;  // you need to enable tests explicitly
 
   {
     argh::parser cmdline;
@@ -42,16 +42,16 @@ int main(int, char **argv) {
     cmdline.parse(argv);
     for (auto &flag : cmdline.flags()) {
       if (flag == "download") {
-        settings.test_suite |= libndt::nettest::download;
+        settings.nettest_flags |= libndt::nettest_flag::download;
         std::clog << "will run download" << std::endl;
       } else if (flag == "download-ext") {
-        settings.test_suite |= libndt::nettest::download_ext;
+        settings.nettest_flags |= libndt::nettest_flag::download_ext;
         std::clog << "will run download-ext" << std::endl;
       } else if (flag == "json") {
-        settings.proto = libndt::protocol::json;
+        settings.protocol_flags = libndt::protocol_flag::json;
         std::clog << "will use json" << std::endl;
       } else if (flag == "upload") {
-        settings.test_suite |= libndt::nettest::upload;
+        settings.nettest_flags |= libndt::nettest_flag::upload;
         std::clog << "will run upload" << std::endl;
       } else if (flag == "verbose") {
         settings.verbosity = libndt::verbosity::debug;

libndt.cpp

@@ -290,7 +290,7 @@ bool Client::query_mlabns() noexcept {
   }
   std::string body;
   if (!query_mlabns_curl(  //
-          impl->settings.mlabns_url, impl->settings.curl_timeout, &body)) {
+          impl->settings.mlabns_url, impl->settings.timeout, &body)) {
     return false;
   }
   nlohmann::json json;
@@ -383,20 +383,20 @@ bool Client::recv_tests_ids() noexcept {
 bool Client::run_tests() noexcept {
   for (auto &tid : impl->granted_suite) {
     switch (tid) {
-      case nettest::upload:
+      case nettest_flag::upload:
         EMIT_INFO("running upload test");
         if (!run_upload()) {
           return false;
         }
         break;
-      case nettest::meta:
+      case nettest_flag::meta:
         EMIT_DEBUG("running meta test");  // don't annoy the user with this
         if (!run_meta()) {
           return false;
         }
         break;
-      case nettest::download:
-      case nettest::download_ext:
+      case nettest_flag::download:
+      case nettest_flag::download_ext:
         EMIT_INFO("running download test");
         if (!run_download()) {
           return false;
@@ -543,7 +543,7 @@ bool Client::run_download() noexcept {
       std::chrono::duration<double> measurement_interval = now - prev;
       std::chrono::duration<double> elapsed = now - begin;
       if (measurement_interval.count() > 0.25) {
-        on_performance(nettest::download, nflows, recent_data,
+        on_performance(nettest_flag::download, nflows, recent_data,
                        measurement_interval.count(), elapsed.count(),
                        impl->settings.max_runtime);
         recent_data = 0;
@@ -711,7 +711,7 @@ bool Client::run_upload() noexcept {
       std::chrono::duration<double> measurement_interval = now - prev;
       std::chrono::duration<double> elapsed = now - begin;
       if (measurement_interval.count() > 0.25) {
-        on_performance(nettest::upload, nflows, recent_data,
+        on_performance(nettest_flag::upload, nflows, recent_data,
                        measurement_interval.count(), elapsed.count(),
                        impl->settings.max_runtime);
         recent_data = 0;
@@ -750,32 +750,34 @@ bool Client::run_upload() noexcept {
 // Low-level API
 
 bool Client::msg_write_login(const std::string &version) noexcept {
-  static_assert(sizeof(impl->settings.test_suite) == 1, "test_suite too large");
+  static_assert(sizeof(impl->settings.nettest_flags) == 1,
+                "nettest_flags too large");
   uint8_t code = 0;
-  impl->settings.test_suite |= nettest::status | nettest::meta;
-  if ((impl->settings.test_suite & nettest::middlebox)) {
-    EMIT_WARNING("msg_write_login(): nettest::middlebox: not implemented");
-    impl->settings.test_suite &= ~nettest::middlebox;
+  impl->settings.nettest_flags |= nettest_flag::status | nettest_flag::meta;
+  if ((impl->settings.nettest_flags & nettest_flag::middlebox)) {
+    EMIT_WARNING("msg_write_login(): nettest_flag::middlebox: not implemented");
+    impl->settings.nettest_flags &= ~nettest_flag::middlebox;
   }
-  if ((impl->settings.test_suite & nettest::simple_firewall)) {
+  if ((impl->settings.nettest_flags & nettest_flag::simple_firewall)) {
     EMIT_WARNING(
-        "msg_write_login(): nettest::simple_firewall: not implemented");
-    impl->settings.test_suite &= ~nettest::simple_firewall;
+        "msg_write_login(): nettest_flag::simple_firewall: not implemented");
+    impl->settings.nettest_flags &= ~nettest_flag::simple_firewall;
   }
-  if ((impl->settings.test_suite & nettest::upload_ext)) {
-    EMIT_WARNING("msg_write_login(): nettest::upload_ext: not implemented");
-    impl->settings.test_suite &= ~nettest::upload_ext;
+  if ((impl->settings.nettest_flags & nettest_flag::upload_ext)) {
+    EMIT_WARNING(
+        "msg_write_login(): nettest_flag::upload_ext: not implemented");
+    impl->settings.nettest_flags &= ~nettest_flag::upload_ext;
   }
   std::string serio;
-  if ((impl->settings.proto & protocol::json) == 0) {
-    serio = std::string{(char *)&impl->settings.test_suite,
-                        sizeof(impl->settings.test_suite)};
+  if ((impl->settings.protocol_flags & protocol_flag::json) == 0) {
+    serio = std::string{(char *)&impl->settings.nettest_flags,
+                        sizeof(impl->settings.nettest_flags)};
     code = msg_login;
   } else {
     code = msg_extended_login;
     nlohmann::json msg{
         {"msg", version},
-        {"tests", std::to_string((unsigned)impl->settings.test_suite)},
+        {"tests", std::to_string((unsigned)impl->settings.nettest_flags)},
     };
     try {
       serio = msg.dump();
@@ -785,7 +787,7 @@ bool Client::msg_write_login(const std::string &version) noexcept {
     }
   }
   assert(code != 0);
-  if ((impl->settings.proto & protocol::websockets) != 0) {
+  if ((impl->settings.protocol_flags & protocol_flag::websockets) != 0) {
     EMIT_WARNING("msg_write_login: websockets not supported");
     return false;
   }
@@ -800,7 +802,7 @@ bool Client::msg_write_login(const std::string &version) noexcept {
 // a different implementation depending on the actual protocol.
 bool Client::msg_write(uint8_t code, std::string &&msg) noexcept {
   EMIT_DEBUG("msg_write: message to send: " << represent(msg));
-  if ((impl->settings.proto & protocol::json) != 0) {
+  if ((impl->settings.protocol_flags & protocol_flag::json) != 0) {
     nlohmann::json json;
     json["msg"] = msg;
     try {
@@ -810,7 +812,7 @@ bool Client::msg_write(uint8_t code, std::string &&msg) noexcept {
       return false;
     }
   }
-  if ((impl->settings.proto & protocol::websockets) != 0) {
+  if ((impl->settings.protocol_flags & protocol_flag::websockets) != 0) {
     EMIT_WARNING("msg_write: websockets not supported");
     return false;
   }
@@ -944,14 +946,14 @@ bool Client::msg_expect(uint8_t expected_code, std::string *s) noexcept {
 bool Client::msg_read(uint8_t *code, std::string *msg) noexcept {
   assert(code != nullptr && msg != nullptr);
   std::string s;
-  if ((impl->settings.proto & protocol::websockets) != 0) {
+  if ((impl->settings.protocol_flags & protocol_flag::websockets) != 0) {
     EMIT_WARNING("msg_read: websockets not supported");
     return false;
   }
   if (!msg_read_legacy(code, &s)) {
     return false;
   }
-  if ((impl->settings.proto & protocol::json) == 0) {
+  if ((impl->settings.protocol_flags & protocol_flag::json) == 0) {
     std::swap(s, *msg);
   } else {
     nlohmann::json json;

libndt.hpp

@@ -55,21 +55,21 @@ namespace libndt {
 namespace version {
 
 /// Major API version number of measurement-kit/libndt.
-constexpr uint64_t api_major = 0;
+constexpr uint64_t major = 0;
 
 /// Minor API version number of measurement-kit/libndt.
-constexpr uint64_t api_minor = 23;
+constexpr uint64_t minor = 23;
 
 /// Patch API version number of measurement-kit/libndt.
-constexpr uint64_t api_patch = 0;
+constexpr uint64_t patch = 0;
 
 }  // namespace version
 
-/// Contains nettests identifiers. You can run multiple nettests as part of
+/// Contains nettests flags. You can run multiple nettests as part of
 /// a single NDT transaction with a NDT server. To specify what nettests you
-/// want to run, modify Settings::test_suite accordingly, by using the
-/// constants contained inside of this namespace.
-namespace nettest {
+/// want to run, modify Settings::nettest_flags accordingly, by using the
+/// constants flags defined inside of this namespace.
+namespace nettest_flag {
 
 constexpr uint8_t middlebox = 1U << 0;
 
@@ -112,22 +112,21 @@ constexpr uint64_t debug = 3;
 
 constexpr const char *ndt_version_compat = "v3.7.0";
 
-/// Type containing the size of something.
 using Size = uint64_t;
 
-/// Type containing the signed size of something.
 using Ssize = int64_t;
 
-/// Type wide enough to contain a socket.
 using Socket = int64_t;
 
-/// Type wide enough to contain `socklen_t`.
 using SockLen = int;
 
 /// Contains flags definiting what protocol to use. Historically NDT used a
 /// binary, cleartext protocol for communicating with the server. Historically
 /// messages were raw strings framed using the binary framing.
-namespace protocol {
+namespace protocol_flag {
+
+/// Indicates that no protocol flags have been set.
+constexpr uint64_t none = 0;
 
 /// When this flag is set we use JSON messages. This specifically means that
 /// we send and receive JSON messages (as opposed to raw strings).
@@ -153,9 +152,9 @@ class Settings {
   /// hostname, mlab-ns won't be used.
   std::string mlabns_url = "https://mlab-ns.appspot.com/ndt";
 
-  /// cURL timeout used when querying mlab-ns. If you specify an explicit
-  /// hostname, mlab-ns won't be used.
-  long curl_timeout = 3 /* seconds */;
+  /// Timeout used for I/O operations. \bug in v0.23.0 this timeout is only
+  /// used for cURL operations, but this will be fixed in v0.24.0.
+  uint16_t timeout = 3 /* seconds */;
 
   /// Host name of the NDT server to use. If this is left blank (the default),
   /// we will use mlab-ns to discover a nearby server.
@@ -165,7 +164,7 @@ class Settings {
   std::string port = "3001";
 
   /// The tests you want to run with the NDT server.
-  uint8_t test_suite = nettest::download;
+  uint8_t nettest_flags = nettest_flag::download;
 
   /// Verbosity of the client. By default no message is emitted. Set to other
   /// values to get more messages (useful when debugging).
@@ -181,13 +180,13 @@ class Settings {
   /// Type of NDT protocol that you want to use. Depending on the requested
   /// protocol, you may need to change also the port. By default, NDT listens
   /// on port 3001 for in-clear communications and port 3010 for TLS ones.
-  uint64_t proto = 0;
+  uint64_t protocol_flags = protocol_flag::none;
 
   /// Maximum time for which a nettest (i.e. download) is allowed to run. After
   /// this time has elapsed, the code will stop downloading (or uploading). It
   /// is meant as a safeguard to prevent the test for running for much more time
   /// than anticipated, due to buffering and/or changing network conditions.
-  double max_runtime = 14 /* seconds */;
+  uint16_t max_runtime = 14 /* seconds */;
 
   /// SOCKSv5h port to use for tunnelling traffic using, e.g., Tor. If non
   /// empty, all DNS and TCP traffic should be tunnelled over such port.