network: make Request object take Manager*

Previously, even though all Request object are owned by Manager, they
do not have direct reference to Manager, but through Link or NetDev
object. But, as Link or NetDev can be NULL, we need to conditionalize
how to access Manager from Request with the type of the request.
This makes the way simpler, as now Request object has direct reference
to Manager.

This also rename request_drop() -> request_detach(), as in the previous
commit, the reference counter is introduced, so even if a reference of
a Request object from Manager is dropped, the object may still alive.
The naming `request_drop()` sounds the object will freed by the
function. But it may not. And `request_detach()` suggests the object
will not be managed by Manager any more, and I think it is more
appropreate.

This is just a cleanup, and should not change any behavior.

diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c
index 3164f82c304eb090ffe242dc78531182cd6079eb..63b40da4328b1f9dbce3c0b5bae1619bb6644d80 100644 (file)
--- a/src/network/networkd-address.c
+++ b/src/network/networkd-address.c
@@ -1295,7 +1295,7 @@ void address_cancel_request(Address *address) {
                 .address = address,
         };
 
-        request_drop(ordered_set_get(address->link->manager->request_queue, &req));
+        request_detach(address->link->manager, &req);
         address_cancel_requesting(address);
 }
 

diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index 48f8e40fb40107902c73fa9b98353c2f97b96884..e925739d06e5f220c30b5d18408e9034b14ddae9 100644 (file)
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -906,7 +906,7 @@ static void link_drop_requests(Link *link) {
 
         ORDERED_SET_FOREACH(req, link->manager->request_queue)
                 if (req->link == link)
-                        request_drop(req);
+                        request_detach(link->manager, req);
 }
 
 static Link *link_drop(Link *link) {

diff --git a/src/network/networkd-queue.c b/src/network/networkd-queue.c
index 06a0cb066e276a0895bfda2231ab36e29fb02c6f..70edc27fcdf2cfb586597ee132f5226523001f40 100644 (file)
--- a/src/network/networkd-queue.c
+++ b/src/network/networkd-queue.c
@@ -81,25 +81,38 @@ static Request *request_free(Request *req) {
         if (!req)
                 return NULL;
 
-        if (req->link && req->link->manager)
-                /* To prevent from triggering assertions in hash functions, remove this request before
-                 * freeing object below. */
-                ordered_set_remove(req->link->manager->request_queue, req);
+        /* To prevent from triggering assertions in the hash and compare functions, remove this request
+         * before freeing userdata below. */
+        if (req->manager)
+                ordered_set_remove(req->manager->request_queue, req);
+
         if (req->consume_object)
                 request_free_object(req->type, req->object);
-        link_unref(req->link);
+
+        link_unref(req->link); /* link may be NULL, but link_unref() can handle it gracefully. */
 
         return mfree(req);
 }
 
 DEFINE_TRIVIAL_REF_UNREF_FUNC(Request, request, request_free);
 
-void request_drop(Request *req) {
+void request_detach(Manager *manager, Request *req) {
+        assert(manager);
+
         if (!req)
                 return;
 
-        if (req->message_counter)
+        req = ordered_set_remove(manager->request_queue, req);
+        if (!req)
+                return;
+
+        req->manager = NULL;
+
+        if (req->message_counter) {
+                assert(*req->message_counter > 0);
                 (*req->message_counter)--;
+                req->message_counter = NULL; /* To prevent double decrement. */
+        }
 
         request_unref(req);
 }
@@ -257,6 +270,7 @@ int netdev_queue_request(
 
         *req = (Request) {
                 .n_ref = 1,
+                .manager = netdev->manager,
                 .netdev = netdev_ref(netdev),
                 .type = REQUEST_TYPE_NETDEV_INDEPENDENT,
                 .consume_object = true,
@@ -264,9 +278,6 @@ int netdev_queue_request(
 
         existing = ordered_set_get(netdev->manager->request_queue, req);
         if (existing) {
-                /* To prevent from removing the existing request. */
-                req->netdev = netdev_unref(req->netdev);
-
                 if (ret)
                         *ret = existing;
                 return 0;
@@ -276,6 +287,8 @@ int netdev_queue_request(
         if (r < 0)
                 return r;
 
+        req->manager = netdev->manager;
+
         if (ret)
                 *ret = req;
 
@@ -336,9 +349,6 @@ int link_queue_request(
 
         existing = ordered_set_get(link->manager->request_queue, req);
         if (existing) {
-                /* To prevent from removing the existing request. */
-                req->link = link_unref(req->link);
-
                 if (ret)
                         *ret = existing;
                 return 0;
@@ -348,6 +358,7 @@ int link_queue_request(
         if (r < 0)
                 return r;
 
+        req->manager = link->manager;
         if (req->message_counter)
                 (*req->message_counter)++;
 
@@ -440,8 +451,8 @@ int manager_process_requests(sd_event_source *s, void *userdata) {
                                 if (req->link)
                                         link_enter_failed(req->link);
                         } else if (r > 0) {
-                                ordered_set_remove(manager->request_queue, req);
-                                request_unref(req);
+                                req->message_counter = NULL;
+                                request_detach(manager, req);
                                 processed = true;
                         }
                 }

diff --git a/src/network/networkd-queue.h b/src/network/networkd-queue.h
index 7410e5c8612dba9c398d7e90653a5ee730bb34ad..14403efdd084d6e7ec171ebbd8165a7fa88e7a36 100644 (file)
--- a/src/network/networkd-queue.h
+++ b/src/network/networkd-queue.h
@@ -46,7 +46,9 @@ typedef enum RequestType {
 typedef struct Request {
         unsigned n_ref;
 
-        Link *link;
+        Manager *manager; /* must be non-NULL */
+        Link *link; /* can be NULL */
+
         RequestType type;
         bool consume_object;
         union {
@@ -74,7 +76,7 @@ Request *request_ref(Request *req);
 Request *request_unref(Request *req);
 DEFINE_TRIVIAL_CLEANUP_FUNC(Request*, request_unref);
 
-void request_drop(Request *req);
+void request_detach(Manager *manager, Request *req);
 
 int netdev_queue_request(
                 NetDev *netdev,

diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c
index 1c5ace643ed0dd4f657d1c99b60fdbb634822e9a..9be1190655e992776e5dcc8830978a7491ae0ad1 100644 (file)
--- a/src/network/networkd-route.c
+++ b/src/network/networkd-route.c
@@ -1560,7 +1560,7 @@ void route_cancel_request(Route *route, Link *link) {
                 .route = route,
         };
 
-        request_drop(ordered_set_get(link->manager->request_queue, &req));
+        request_detach(link->manager, &req);
         route_cancel_requesting(route);
 }