From 9b4934df18955e7e37b6880d8e748541dd2aad04 Mon Sep 17 00:00:00 2001 From: Arran Cudbard-Bell Date: Tue, 22 Apr 2025 23:30:56 -0500 Subject: [PATCH] Use macros to check return codes of disam/delete functions --- src/lib/curl/io.c | 2 +- src/lib/io/load.c | 3 ++- src/lib/io/master.c | 7 ++----- src/lib/server/connection.c | 5 +---- src/lib/server/trunk.c | 4 +--- src/lib/unlang/xlat.c | 8 ++------ src/lib/util/timer.c | 4 ++-- src/lib/util/timer.h | 8 +++++++- src/modules/rlm_radius/bio.c | 12 ++---------- src/modules/rlm_unbound/io.c | 10 +++------- 10 files changed, 23 insertions(+), 40 deletions(-) diff --git a/src/lib/curl/io.c b/src/lib/curl/io.c index 8c70638157..98d02a345a 100644 --- a/src/lib/curl/io.c +++ b/src/lib/curl/io.c @@ -277,7 +277,7 @@ static int _fr_curl_io_timer_modify(CURLM *mandle, long timeout_ms, void *ctx) fr_curl_handle_t *mhandle = talloc_get_type_abort(ctx, fr_curl_handle_t); if (timeout_ms < 0) { - if (!fr_cond_assert_msg(fr_timer_disarm(mhandle->ev) == 0, "Failed disarming curl timer")) return -1; + FR_TIMER_DISARM_RETURN(mhandle->ev); DEBUG3("multi-handle %p - Timer removed", mandle); return 0; } diff --git a/src/lib/io/load.c b/src/lib/io/load.c index 760209b157..ab429934a5 100644 --- a/src/lib/io/load.c +++ b/src/lib/io/load.c @@ -253,7 +253,8 @@ int fr_load_generator_stop(fr_load_t *l) { if (!fr_timer_armed(l->ev)) return 0; - return fr_timer_delete(&l->ev); + FR_TIMER_DELETE_RETURN(&l->ev); + return 0; } diff --git a/src/lib/io/master.c b/src/lib/io/master.c index 4e14b350a4..c3a55ea58e 100644 --- a/src/lib/io/master.c +++ b/src/lib/io/master.c @@ -179,7 +179,7 @@ static fr_event_update_t resume_read[] = { static int track_free(fr_io_track_t *track) { - if (fr_cond_assert_msg(fr_timer_delete(&track->ev) == 0, "failed deleting tracking timer")) return -1; + FR_TIMER_DELETE_RETURN(&track->ev); talloc_free_children(track); fr_assert(track->client->packets > 0); @@ -1817,10 +1817,7 @@ have_client: * want to clean it up. */ if (fr_timer_armed(client->ev)) { - if (fr_cond_assert_msg(fr_timer_delete(&client->ev) == 0, - "failed deleting client timer")) { - return -1; - } + FR_TIMER_DELETE_RETURN(&client->ev); client->ready_to_delete = false; } diff --git a/src/lib/server/connection.c b/src/lib/server/connection.c index 123e8e82ae..7524eeb831 100644 --- a/src/lib/server/connection.c +++ b/src/lib/server/connection.c @@ -1453,10 +1453,7 @@ static int _connection_free(connection_t *conn) /* * Explicitly cancel any pending events */ - if (!fr_cond_assert_msg(fr_timer_delete(&conn->ev) == 0, "failed deleting connection timer")) { - return -1; - } - + FR_TIMER_DELETE_RETURN(&conn->ev); /* * Don't allow the connection to be * arbitrarily freed by a callback. diff --git a/src/lib/server/trunk.c b/src/lib/server/trunk.c index b562000f87..b29bba6f50 100644 --- a/src/lib/server/trunk.c +++ b/src/lib/server/trunk.c @@ -4873,9 +4873,7 @@ static int _trunk_free(trunk_t *trunk) * We really don't want this firing after * we've freed everything. */ - if (!fr_cond_assert_msg(fr_timer_delete(&trunk->manage_ev) == 0, "failed deleting trunk management event")) { - return -1; - } + FR_TIMER_DELETE_RETURN(&trunk->manage_ev); /* * Now free the connections in each of the lists. diff --git a/src/lib/unlang/xlat.c b/src/lib/unlang/xlat.c index 29daa3a71d..6cd9d72798 100644 --- a/src/lib/unlang/xlat.c +++ b/src/lib/unlang/xlat.c @@ -96,9 +96,7 @@ typedef struct { */ static int _unlang_xlat_event_free(unlang_xlat_event_t *ev) { - if (!fr_cond_assert_msg(fr_timer_delete(&(ev->ev)) == 0, "failed freeing xlat event timer")) { - return -1; - } + FR_TIMER_DELETE(&(ev->ev)); if (ev->fd >= 0) { (void) fr_event_fd_delete(unlang_interpret_event_list(ev->request), ev->fd, FR_EVENT_FILTER_IO); @@ -597,9 +595,7 @@ xlat_action_t unlang_xlat_yield(request_t *request, */ static int _unlang_xlat_retry_free(unlang_xlat_retry_t *ev) { - if (!fr_cond_assert_msg(fr_timer_delete(&(ev->ev)) == 0, "failed to deleting xlat retry timer")) { - return -1; - } + FR_TIMER_DELETE(&(ev->ev)); return 0; } diff --git a/src/lib/util/timer.c b/src/lib/util/timer.c index 07d70f61b2..b7638f1551 100644 --- a/src/lib/util/timer.c +++ b/src/lib/util/timer.c @@ -267,7 +267,7 @@ static inline CC_HINT(always_inline) int timer_list_parent_update(fr_timer_list_ /* * Disables the timer in the parent, does not free the memory */ - if (tl->parent) if (unlikely(fr_timer_disarm(tl->parent_ev) < 0)) return -1; + if (tl->parent) FR_TIMER_DISARM_RETURN(tl->parent_ev); return 0; } @@ -1007,7 +1007,7 @@ static int _timer_list_free(fr_timer_list_t *tl) return -1; } - if (tl->parent_ev) fr_timer_delete(&tl->parent_ev); + if (tl->parent_ev) if (unlikely(fr_timer_delete(&tl->parent_ev) < 0)) return -1; while ((ev = timer_funcs[tl->type].head(tl))) { if (talloc_free(ev) < 0) return -1; diff --git a/src/lib/util/timer.h b/src/lib/util/timer.h index a540eaf9e8..476e68a365 100644 --- a/src/lib/util/timer.h +++ b/src/lib/util/timer.h @@ -94,15 +94,21 @@ int fr_timer_disarm(fr_timer_t *ev); /* disarms but does not free */ } \ } while (0) +#define FR_TIMER_DISARM_RETURN(_ev) \ + if ((likely(((_ev)) != NULL) && unlikely(!fr_cond_assert_msg(fr_timer_disarm(_ev) == 0, "Failed to disarm timer %p", (_ev))))) return -1; + int fr_timer_delete(fr_timer_t **ev_p) CC_HINT(nonnull); /* disarms AND frees */ #define FR_TIMER_DELETE(_ev_p) \ do { \ if ((likely((*(_ev_p)) != NULL) && unlikely(fr_timer_delete(_ev_p) < 0))) { \ - fr_assert_msg(0, "Failed to delete timer %p", (_ev_p)); \ + fr_assert_msg(0, "Failed to delete timer %p", *(_ev_p)); \ } \ } while (0) +#define FR_TIMER_DELETE_RETURN(_ev_p) \ + if ((likely((*(_ev_p)) != NULL) && unlikely(!fr_cond_assert_msg(fr_timer_delete(_ev_p) == 0, "Failed to delete timer %p", *(_ev_p))))) return -1; + fr_time_t fr_timer_when(fr_timer_t *ev) CC_HINT(nonnull); bool _fr_timer_armed(fr_timer_t *ev); diff --git a/src/modules/rlm_radius/bio.c b/src/modules/rlm_radius/bio.c index 9e19e53370..e3abb77f21 100644 --- a/src/modules/rlm_radius/bio.c +++ b/src/modules/rlm_radius/bio.c @@ -613,12 +613,7 @@ static int _bio_handle_free(bio_handle_t *h) fr_assert(h->fd >= 0); - if (h->status_u) { - if (!fr_cond_assert_msg(fr_timer_delete(&h->status_u->ev) == 0, - "failed deleting status check timer")) { - return -1; - } - } + if (h->status_u) FR_TIMER_DELETE_RETURN(&h->status_u->ev); /* * The connection code will take care of deleting the FD from the event loop. @@ -2252,10 +2247,7 @@ static int _bio_request_free(bio_request_t *u) fr_assert_msg(!fr_timer_armed(u->ev), "bio_request_t freed with active timer"); - if (!fr_cond_assert_msg(fr_timer_delete(&u->ev) == 0, - "failed deleting request timer")) { - return -1; - } + FR_TIMER_DELETE_RETURN(&u->ev); fr_assert(u->rr == NULL); diff --git a/src/modules/rlm_unbound/io.c b/src/modules/rlm_unbound/io.c index c790b42f8f..71cdf304bf 100644 --- a/src/modules/rlm_unbound/io.c +++ b/src/modules/rlm_unbound/io.c @@ -346,9 +346,7 @@ static int _unbound_io_event_deactivate(struct ub_event *ub_ev) if (ev->events & UB_EV_TIMEOUT) { DEBUG4("unbound event %p - Disarming timeout", ev); - if (!fr_cond_assert_msg(fr_timer_disarm(ev->timer_ev) == 0, "failed disarming timeout")) { - ret = -1; - } + FR_TIMER_DISARM_RETURN(ev->timer_ev); } ev->active = false; /* Event is now inactive and can be modified */ @@ -379,9 +377,7 @@ static int _unbound_io_timer_modify(struct ub_event *ub_ev, UNUSED struct ub_eve ev, uctx, ev->uctx); ev->uctx = uctx; } - if (!fr_cond_assert_msg(fr_timer_disarm(ev->timer_ev) == 0, "ubound event %p - Failed disarming timeout", ev)) { - ret = -1; /* Continue ? */ - } + FR_TIMER_DISARM_RETURN(ev->timer_ev); timeout = fr_time_delta_from_timeval(tv); @@ -409,7 +405,7 @@ static int _unbound_io_timer_deactivate(struct ub_event *ub_ev) DEBUG4("unbound event %p - Disarming timeout", ev); - if (!fr_cond_assert_msg(fr_timer_disarm(ev->timer_ev) == 0, "failed disarming timeout")) return -1; + FR_TIMER_DISARM_RETURN(ev->timer_ev); return 0; } -- 2.47.3