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.
This commit is contained in:
Yu Watanabe
2022-02-27 15:39:16 +09:00
parent e9ef9a1484
commit e26d3d407c
5 changed files with 33 additions and 20 deletions

View File

@@ -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);
}

View File

@@ -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) {

View File

@@ -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;
}
}

View File

@@ -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,

View File

@@ -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);
}