Merge pull request #3287 from canonical/small-tweaks

Small code tweaks

include/multipass/memory_size.h

@@ -25,12 +25,12 @@ namespace multipass
 class MemorySize
 {
 public:
-    friend bool operator==(const MemorySize& a, const MemorySize& b);
-    friend bool operator!=(const MemorySize& a, const MemorySize& b);
-    friend bool operator<(const MemorySize& a, const MemorySize& b);
-    friend bool operator>(const MemorySize& a, const MemorySize& b);
-    friend bool operator<=(const MemorySize& a, const MemorySize& b);
-    friend bool operator>=(const MemorySize& a, const MemorySize& b);
+    friend bool operator==(const MemorySize& a, const MemorySize& b) noexcept;
+    friend bool operator!=(const MemorySize& a, const MemorySize& b) noexcept;
+    friend bool operator<(const MemorySize& a, const MemorySize& b) noexcept;
+    friend bool operator>(const MemorySize& a, const MemorySize& b) noexcept;
+    friend bool operator<=(const MemorySize& a, const MemorySize& b) noexcept;
+    friend bool operator>=(const MemorySize& a, const MemorySize& b) noexcept;
 
     MemorySize() noexcept;
     explicit MemorySize(const std::string& val);
@@ -45,12 +45,12 @@ class MemorySize
     long long bytes;
 };
 
-bool operator==(const MemorySize& a, const MemorySize& b);
-bool operator!=(const MemorySize& a, const MemorySize& b);
-bool operator<(const MemorySize& a, const MemorySize& b);
-bool operator>(const MemorySize& a, const MemorySize& b);
-bool operator<=(const MemorySize& a, const MemorySize& b);
-bool operator>=(const MemorySize& a, const MemorySize& b);
+bool operator==(const MemorySize& a, const MemorySize& b) noexcept;
+bool operator!=(const MemorySize& a, const MemorySize& b) noexcept;
+bool operator<(const MemorySize& a, const MemorySize& b) noexcept;
+bool operator>(const MemorySize& a, const MemorySize& b) noexcept;
+bool operator<=(const MemorySize& a, const MemorySize& b) noexcept;
+bool operator>=(const MemorySize& a, const MemorySize& b) noexcept;
 
 } // namespace multipass
 

src/client/cli/cmd/delete.cpp

@@ -71,7 +71,6 @@ mp::ReturnCode cmd::Delete::run(mp::ArgParser* parser)
 
     auto on_failure = [this](grpc::Status& status) { return standard_failure_handler_for(name(), cerr, status); };
 
-    request.set_verbosity_level(parser->verbosityLevel());
     return dispatch(&RpcMethod::delet, request, on_success, on_failure);
 }
 
@@ -112,6 +111,7 @@ mp::ParseCode cmd::Delete::parse_args(mp::ArgParser* parser)
         return status;
 
     request.set_purge(parser->isSet(purge_option));
+    request.set_verbosity_level(parser->verbosityLevel());
 
     status = parse_instances_snapshots(parser);
 

src/client/cli/cmd/restore.cpp

@@ -46,7 +46,7 @@ mp::ReturnCode cmd::Restore::run(mp::ArgParser* parser)
     };
 
     using Client = grpc::ClientReaderWriterInterface<RestoreRequest, RestoreReply>;
-    auto streaming_callback = [this, &spinner](mp::RestoreReply& reply, Client* client) {
+    auto streaming_callback = [this, &spinner](const mp::RestoreReply& reply, Client* client) {
         if (!reply.log_line().empty())
             spinner.print(cerr, reply.log_line());
 

src/daemon/daemon.cpp

@@ -1131,8 +1131,8 @@ InstanceSnapshotsMap map_snapshots_to_instances(const InstanceSnapshotPairs& ins
 void verify_snapshot_picks(const InstanceSelectionReport& report,
                            const std::unordered_map<std::string, SnapshotPick>& snapshot_picks)
 {
-    for (const auto& selection : {report.deleted_selection, report.operative_selection})
-        for (const auto& vm_it : selection)
+    for (const auto* selection : {&report.deleted_selection, &report.operative_selection})
+        for (const auto& vm_it : *selection)
             if (auto pick_it = snapshot_picks.find(vm_it->first); pick_it != snapshot_picks.end())
                 for (const auto& snapshot_name : pick_it->second.pick)
                     vm_it->second->get_snapshot(snapshot_name); // throws if it doesn't exist
@@ -2234,9 +2234,9 @@ try // clang-format on
         verify_snapshot_picks(instance_selection, instance_snapshots_map); // avoid deleting if any snapshot is missing
 
         // start with deleted instances, to avoid iterator invalidation when moving instances there
-        for (const auto& selection : {instance_selection.deleted_selection, instance_selection.operative_selection})
+        for (const auto* selection : {&instance_selection.deleted_selection, &instance_selection.operative_selection})
         {
-            for (const auto& vm_it : selection)
+            for (const auto& vm_it : *selection)
             {
                 const auto& instance_name = vm_it->first;
 

src/daemon/instance_settings_handler.cpp

@@ -181,7 +181,7 @@ std::set<QString> mp::InstanceSettingsHandler::keys() const
 
     std::set<QString> ret;
     for (const auto& item : vm_instance_specs)
-        for (const auto& suffix : {cpus_suffix, mem_suffix, disk_suffix})
+        for (const auto* suffix : {cpus_suffix, mem_suffix, disk_suffix})
             ret.insert(key_template.arg(item.first.c_str(), suffix));
 
     return ret;

src/daemon/snapshot_settings_handler.cpp

@@ -96,7 +96,7 @@ std::set<QString> mp::SnapshotSettingsHandler::keys() const
     for (const auto* instance_map : {&const_operative_instances, &deleted_instances})
         for (const auto& [vm_name, vm] : *instance_map)
             for (const auto& snapshot : vm->view_snapshots())
-                for (const auto& suffix : {name_suffix, comment_suffix})
+                for (const auto* suffix : {name_suffix, comment_suffix})
                     ret.insert(key_template.arg(vm_name.c_str(), snapshot->get_name().c_str(), suffix));
 
     return ret;

src/platform/backends/shared/linux/backend_utils.cpp

@@ -157,9 +157,9 @@ auto make_bridge_rollback_guard(const mpl::CString& log_category, const mpdbus::
                                 const QDBusObjectPath& parent_path, const QDBusObjectPath& child_path)
 {
     auto rollback = [&system_bus, &parent_path, &child_path] {
-        for (auto& obj_path : {child_path, parent_path})
+        for (const auto* obj_path : {&child_path, &parent_path})
         {
-            if (auto path = obj_path.path(); !path.isNull())
+            if (auto path = obj_path->path(); !path.isNull())
             {
                 auto connection = system_bus.get_interface(nm_bus_name, path, nm_connection_ifc);
                 checked_dbus_call<void, /* RollingBack = */ true>(*connection, "Delete");

src/utils/memory_size.cpp

@@ -107,32 +107,32 @@ long long mp::MemorySize::in_gigabytes() const noexcept
     return bytes / gibi; // integer division to floor
 }
 
-bool mp::operator==(const MemorySize& a, const MemorySize& b)
+bool mp::operator==(const MemorySize& a, const MemorySize& b) noexcept
 {
     return a.bytes == b.bytes;
 }
 
-bool mp::operator!=(const MemorySize& a, const MemorySize& b)
+bool mp::operator!=(const MemorySize& a, const MemorySize& b) noexcept
 {
     return a.bytes != b.bytes;
 }
 
-bool mp::operator<(const MemorySize& a, const MemorySize& b)
+bool mp::operator<(const MemorySize& a, const MemorySize& b) noexcept
 {
     return a.bytes < b.bytes;
 }
 
-bool mp::operator>(const MemorySize& a, const MemorySize& b)
+bool mp::operator>(const MemorySize& a, const MemorySize& b) noexcept
 {
     return a.bytes > b.bytes;
 }
 
-bool mp::operator<=(const MemorySize& a, const MemorySize& b)
+bool mp::operator<=(const MemorySize& a, const MemorySize& b) noexcept
 {
     return a.bytes <= b.bytes;
 }
 
-bool mp::operator>=(const MemorySize& a, const MemorySize& b)
+bool mp::operator>=(const MemorySize& a, const MemorySize& b) noexcept
 {
     return a.bytes >= b.bytes;
 }

tests/linux/test_platform_linux.cpp

@@ -182,14 +182,14 @@ struct PlatformLinux : public mpt::TestWithMockedBinPath
 
 TEST_F(PlatformLinux, test_interpretation_of_winterm_setting_not_supported)
 {
-    for (const auto x : {"no", "matter", "what"})
+    for (const auto* x : {"no", "matter", "what"})
         EXPECT_THROW(mp::platform::interpret_setting(mp::winterm_key, x), mp::InvalidSettingException);
 }
 
 TEST_F(PlatformLinux, test_interpretation_of_unknown_settings_not_supported)
 {
-    for (const auto k : {"unimaginable", "katxama", "katxatxa"})
-        for (const auto v : {"no", "matter", "what"})
+    for (const auto* k : {"unimaginable", "katxama", "katxatxa"})
+        for (const auto* v : {"no", "matter", "what"})
             EXPECT_THROW(mp::platform::interpret_setting(k, v), mp::InvalidSettingException);
 }
 

tests/test_daemon.cpp

@@ -1836,8 +1836,8 @@ TEST_F(Daemon, does_not_hold_on_to_macs_when_loading_fails)
 
     mp::Daemon daemon{config_builder.build()};
 
-    for (const auto& mac : {mac1, mac2})
-        send_command({"launch", "--network", fmt::format("name=eth0,mac={}", mac)});
+    for (const auto* mac : {&mac1, &mac2})
+        send_command({"launch", "--network", fmt::format("name=eth0,mac={}", *mac)});
 }
 
 TEST_F(Daemon, does_not_hold_on_to_macs_when_image_preparation_fails)

tests/test_instance_settings_handler.cpp

@@ -204,7 +204,7 @@ TEST_F(TestInstanceSettingsHandler, getFetchesPropertiesOfInstanceInSpecialState
 
     const auto handler = make_handler();
 
-    for (const auto& instance : {preparing_instance, deleted_instance})
+    for (const auto* instance : {preparing_instance, deleted_instance})
     {
         for (const auto& property : properties)
         {
@@ -249,7 +249,7 @@ there) */
     spec.ssh_username = "hugo";
     spec.default_mac_address = "+++++";
 
-    for (const auto& name : {"toto", "tata", "fuzz"})
+    for (const auto* name : {"toto", "tata", "fuzz"})
     {
         vms[name];
         specs.emplace(name, spec);