From 3852f94de9582dc1acb44844579873cd0e2f3162 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>

Date: Tue, 11 Jan 2022 15:12:42 +0100

Subject: [PATCH] networkctl: open the bus just once

We'd connect to the bus twice: the first time to check networkd namespace,

and then the second time to do the deed we were asked to do. It's nicer

to open the bus just once, for efficience and also to avoid the open call

in all functions.

An ASSERT_PTR helper is added:

- sd_bus *bus = userdata;

  ...

- assert(bus);

+ sd_bus *bus = ASSERT_PTR(userdata);

  ...

It can be used in other place too, but I'm leaving that for a later

refactoring.

(cherry picked from commit d821e40ca96d2b14216f7a18e4512364bfb83628)

Related: #2087652

---

 src/fundamental/macro-fundamental.h |  8 ++++

 src/network/networkctl.c            | 74 ++++++++++-------------------

 2 files changed, 33 insertions(+), 49 deletions(-)

diff --git a/src/fundamental/macro-fundamental.h b/src/fundamental/macro-fundamental.h

index f87839d47b..d597c743bb 100644

--- a/src/fundamental/macro-fundamental.h

+++ b/src/fundamental/macro-fundamental.h

@@ -66,6 +66,14 @@

         #define free(a) FreePool(a)

 #endif

+/* This passes the argument through after (if asserts are enabled) checking that it is not null. */

+#define ASSERT_PTR(expr)                        \

+        ({                                      \

+                typeof(expr) _expr_ = (expr);   \

+                assert(_expr_);                 \

+                _expr_;                         \

+        })

+

 #if defined(static_assert)

 #define assert_cc(expr)                                                 \

         static_assert(expr, #expr)

diff --git a/src/network/networkctl.c b/src/network/networkctl.c

index 68dd4b185c..c35f851bdb 100644

--- a/src/network/networkctl.c

+++ b/src/network/networkctl.c

@@ -79,17 +79,12 @@ static bool arg_full = false;

 static unsigned arg_lines = 10;

 static JsonFormatFlags arg_json_format_flags = JSON_FORMAT_OFF;

-static int get_description(JsonVariant **ret) {

+static int get_description(sd_bus *bus, JsonVariant **ret) {

         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;

         _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;

-        _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;

         const char *text = NULL;

         int r;

-        r = sd_bus_open_system(&bus;;

-        if (r < 0)

-                return log_error_errno(r, "Failed to connect system bus: %m");

-

         r = bus_call_method(bus, bus_network_mgr, "Describe", &error, &reply, NULL);

         if (r < 0)

                 return log_error_errno(r, "Failed to get description: %s", bus_error_message(&error, r));

@@ -105,11 +100,11 @@ static int get_description(JsonVariant **ret) {

         return 0;

 }

-static int dump_manager_description(void) {

+static int dump_manager_description(sd_bus *bus) {

         _cleanup_(json_variant_unrefp) JsonVariant *v = NULL;

         int r;

-        r = get_description(&v);

+        r = get_description(bus, &v);

         if (r < 0)

                 return r;

@@ -117,14 +112,14 @@ static int dump_manager_description(void) {

         return 0;

 }

-static int dump_link_description(char **patterns) {

+static int dump_link_description(sd_bus *bus, char **patterns) {

         _cleanup_(json_variant_unrefp) JsonVariant *v = NULL;

         _cleanup_free_ bool *matched_patterns = NULL;

         JsonVariant *i;

         size_t c = 0;

         int r;

-        r = get_description(&v);

+        r = get_description(bus, &v);

         if (r < 0)

                 return r;

@@ -790,6 +785,7 @@ static int acquire_link_info(sd_bus *bus, sd_netlink *rtnl, char **patterns, Lin

 }

 static int list_links(int argc, char *argv[], void *userdata) {

+        sd_bus *bus = ASSERT_PTR(userdata);

         _cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL;

         _cleanup_(link_info_array_freep) LinkInfo *links = NULL;

         _cleanup_(table_unrefp) Table *table = NULL;

@@ -798,9 +794,9 @@ static int list_links(int argc, char *argv[], void *userdata) {

         if (arg_json_format_flags != JSON_FORMAT_OFF) {

                 if (arg_all || argc <= 1)

-                        return dump_manager_description();

+                        return dump_manager_description(bus);

                 else

-                        return dump_link_description(strv_skip(argv, 1));

+                        return dump_link_description(bus, strv_skip(argv, 1));

         }

         r = sd_netlink_open(&rtnl);

@@ -2383,7 +2379,7 @@ static int system_status(sd_netlink *rtnl, sd_hwdb *hwdb) {

 }

 static int link_status(int argc, char *argv[], void *userdata) {

-        _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;

+        sd_bus *bus = ASSERT_PTR(userdata);

         _cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL;

         _cleanup_(sd_hwdb_unrefp) sd_hwdb *hwdb = NULL;

         _cleanup_(link_info_array_freep) LinkInfo *links = NULL;

@@ -2391,17 +2387,13 @@ static int link_status(int argc, char *argv[], void *userdata) {

         if (arg_json_format_flags != JSON_FORMAT_OFF) {

                 if (arg_all || argc <= 1)

-                        return dump_manager_description();

+                        return dump_manager_description(bus);

                 else

-                        return dump_link_description(strv_skip(argv, 1));

+                        return dump_link_description(bus, strv_skip(argv, 1));

         }

         pager_open(arg_pager_flags);

-        r = sd_bus_open_system(&bus;;

-        if (r < 0)

-                return log_error_errno(r, "Failed to connect system bus: %m");

-

         r = sd_netlink_open(&rtnl);

         if (r < 0)

                 return log_error_errno(r, "Failed to connect to netlink: %m");

@@ -2738,14 +2730,10 @@ static int link_renew_one(sd_bus *bus, int index, const char *name) {

 }

 static int link_renew(int argc, char *argv[], void *userdata) {

-        _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;

+        sd_bus *bus = ASSERT_PTR(userdata);

         _cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL;

         int index, k = 0, r;

-        r = sd_bus_open_system(&bus;;

-        if (r < 0)

-                return log_error_errno(r, "Failed to connect system bus: %m");

-

         for (int i = 1; i < argc; i++) {

                 index = rtnl_resolve_interface_or_warn(&rtnl, argv[i]);

                 if (index < 0)

@@ -2772,14 +2760,10 @@ static int link_force_renew_one(sd_bus *bus, int index, const char *name) {

 }

 static int link_force_renew(int argc, char *argv[], void *userdata) {

-        _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;

+        sd_bus *bus = ASSERT_PTR(userdata);

         _cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL;

         int k = 0, r;

-        r = sd_bus_open_system(&bus;;

-        if (r < 0)

-                return log_error_errno(r, "Failed to connect system bus: %m");

-

         for (int i = 1; i < argc; i++) {

                 int index = rtnl_resolve_interface_or_warn(&rtnl, argv[i]);

                 if (index < 0)

@@ -2794,14 +2778,10 @@ static int link_force_renew(int argc, char *argv[], void *userdata) {

 }

 static int verb_reload(int argc, char *argv[], void *userdata) {

+        sd_bus *bus = ASSERT_PTR(userdata);

         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;

-        _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;

         int r;

-        r = sd_bus_open_system(&bus;;

-        if (r < 0)

-                return log_error_errno(r, "Failed to connect system bus: %m");

-

         r = bus_call_method(bus, bus_network_mgr, "Reload", &error, NULL, NULL);

         if (r < 0)

                 return log_error_errno(r, "Failed to reload network settings: %m");

@@ -2810,17 +2790,13 @@ static int verb_reload(int argc, char *argv[], void *userdata) {

 }

 static int verb_reconfigure(int argc, char *argv[], void *userdata) {

+        sd_bus *bus = ASSERT_PTR(userdata);

         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;

-        _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;

         _cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL;

         _cleanup_set_free_ Set *indexes = NULL;

         int index, r;

         void *p;

-        r = sd_bus_open_system(&bus;;

-        if (r < 0)

-                return log_error_errno(r, "Failed to connect system bus: %m");

-

         indexes = set_new(NULL);

         if (!indexes)

                 return log_oom();

@@ -2968,7 +2944,7 @@ static int parse_argv(int argc, char *argv[]) {

         return 1;

 }

-static int networkctl_main(int argc, char *argv[]) {

+static int networkctl_main(sd_bus *bus, int argc, char *argv[]) {

         static const Verb verbs[] = {

                 { "list",        VERB_ANY, VERB_ANY, VERB_DEFAULT, list_links          },

                 { "status",      VERB_ANY, VERB_ANY, 0,            link_status         },

@@ -2984,20 +2960,15 @@ static int networkctl_main(int argc, char *argv[]) {

                 {}

         };

-        return dispatch_verb(argc, argv, verbs, NULL);

+        return dispatch_verb(argc, argv, verbs, bus);

 }

-static int check_netns_match(void) {

+static int check_netns_match(sd_bus *bus) {

         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;

-        _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;

         struct stat st;

         uint64_t id;

         int r;

-        r = sd_bus_open_system(&bus;;

-        if (r < 0)

-                return log_error_errno(r, "Failed to connect system bus: %m");

-

         r = sd_bus_get_property_trivial(

                         bus,

                         "org.freedesktop.network1",

@@ -3035,6 +3006,7 @@ static void warn_networkd_missing(void) {

 }

 static int run(int argc, char* argv[]) {

+        _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;

         int r;

         log_setup();

@@ -3043,13 +3015,17 @@ static int run(int argc, char* argv[]) {

         if (r <= 0)

                 return r;

-        r = check_netns_match();

+        r = sd_bus_open_system(&bus;;

+        if (r < 0)

+                return log_error_errno(r, "Failed to connect system bus: %m");

+

+        r = check_netns_match(bus);

         if (r < 0)

                 return r;

         warn_networkd_missing();

-        return networkctl_main(argc, argv);

+        return networkctl_main(bus, argc, argv);

 }

 DEFINE_MAIN_FUNCTION(run);