diff --git a/bin/hooks/filter-aaaa.c b/bin/hooks/filter-aaaa.c index fa964fab69..174725eac2 100644 --- a/bin/hooks/filter-aaaa.c +++ b/bin/hooks/filter-aaaa.c @@ -68,37 +68,37 @@ static isc_mempool_t *datapool = NULL; static bool filter_qctx_initialize(void *hookdata, void *cbdata, isc_result_t *resp); static ns_hook_t filter_init = { - .callback = filter_qctx_initialize, + .action = filter_qctx_initialize, }; static bool filter_respond_begin(void *hookdata, void *cbdata, isc_result_t *resp); static ns_hook_t filter_respbegin = { - .callback = filter_respond_begin, + .action = filter_respond_begin, }; static bool filter_respond_any_found(void *hookdata, void *cbdata, isc_result_t *resp); static ns_hook_t filter_respanyfound = { - .callback = filter_respond_any_found, + .action = filter_respond_any_found, }; static bool filter_prep_response_begin(void *hookdata, void *cbdata, isc_result_t *resp); static ns_hook_t filter_prepresp = { - .callback = filter_prep_response_begin, + .action = filter_prep_response_begin, }; static bool filter_query_done_send(void *hookdata, void *cbdata, isc_result_t *resp); static ns_hook_t filter_donesend = { - .callback = filter_query_done_send, + .action = filter_query_done_send, }; static bool filter_qctx_destroy(void *hookdata, void *cbdata, isc_result_t *resp); ns_hook_t filter_destroy = { - .callback = filter_qctx_destroy, + .action = filter_qctx_destroy, }; /** diff --git a/lib/ns/include/ns/hooks.h b/lib/ns/include/ns/hooks.h index 95f9aca183..760292f0cc 100644 --- a/lib/ns/include/ns/hooks.h +++ b/lib/ns/include/ns/hooks.h @@ -165,6 +165,7 @@ typedef enum { NS_QUERY_QCTX_INITIALIZED, NS_QUERY_QCTX_DESTROYED, + NS_QUERY_SETUP, NS_QUERY_START_BEGIN, NS_QUERY_LOOKUP_BEGIN, NS_QUERY_RESUME_BEGIN, @@ -189,11 +190,11 @@ typedef enum { } ns_hookpoint_t; typedef bool -(*ns_hook_cb_t)(void *hook_data, void *callback_data, isc_result_t *resultp); +(*ns_hook_action_t)(void *arg, void *data, isc_result_t *resultp); typedef struct ns_hook { - ns_hook_cb_t callback; - void *callback_data; + ns_hook_action_t action; + void *action_data; ISC_LINK(struct ns_hook) link; } ns_hook_t; @@ -278,35 +279,6 @@ typedef int ns_hook_version_t(unsigned int *flags); * the future to pass back driver capabilities or other information. */ -/* - * Run a hook. Calls the function or functions registered at hookpoint 'id'. - * If one of them returns true, we interrupt processing and return the - * result that was returned by the hook function. If none of them return - * true, we continue processing. - */ -#define _NS_PROCESS_HOOK(table, id, data, ...) \ - if (table != NULL) { \ - ns_hook_t *_hook = ISC_LIST_HEAD((*table)[id]); \ - isc_result_t _result; \ - \ - while (_hook != NULL) { \ - ns_hook_cb_t _callback = _hook->callback; \ - void *_callback_data = _hook->callback_data; \ - if (_callback != NULL && \ - _callback(data, _callback_data, &_result)) \ - { \ - return __VA_ARGS__; \ - } else { \ - _hook = ISC_LIST_NEXT(_hook, link); \ - } \ - } \ - } - -#define NS_PROCESS_HOOK(table, id, data, ...) \ - _NS_PROCESS_HOOK(table, id, data, _result) -#define NS_PROCESS_HOOK_VOID(table, id, data, ...) \ - _NS_PROCESS_HOOK(table, id, data) - isc_result_t ns_hook_createctx(isc_mem_t *mctx, ns_hookctx_t **hctxp); diff --git a/lib/ns/query.c b/lib/ns/query.c index dcfa5cc368..5de1f2b1f2 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -238,30 +238,73 @@ static void log_noexistnodata(void *val, int level, const char *fmt, ...) ISC_FORMAT_PRINTF(3, 4); -#define PROCESS_HOOK(_id, _qctx, ...) \ + +/* + * Return the hooktable in use with 'qctx', or if there isn't one + * set, return the default hooktable. + */ +static inline ns_hooktable_t * +get_hooktab(query_ctx_t *qctx) { + if (qctx == NULL || qctx->view == NULL || + qctx->view->hooktable == NULL) + { + return (ns__hook_table); + } + + return (qctx->view->hooktable); +} + +/* + * Call the specified hook function in every configured module that + * implements that function. If any hook function returns 'true', we set + * 'result' and terminate processing by jumping to the 'cleanup' tag. + * + * (Note that a hook function may set the 'result' to ISC_R_SUCCESS but + * still terminate processing within the calling function. That's why this + * is a macro instead of an inline function; it needs to be able to use + * 'goto cleanup' regardless of the return value.) + */ +#define PROCESS_HOOK(_id, _qctx) \ do { \ - ns_hooktable_t *_tab = ns__hook_table; \ - query_ctx_t *_q = (_qctx); \ - if (_q != NULL && \ - _q->view != NULL && \ - _q->view->hooktable != NULL) \ - { \ - _tab = _q->view->hooktable; \ + isc_result_t _res; \ + ns_hooktable_t *_tab = get_hooktab(_qctx); \ + ns_hook_t *_hook; \ + _hook = ISC_LIST_HEAD((*_tab)[_id]); \ + while (_hook != NULL) { \ + ns_hook_action_t _func = _hook->action; \ + void *_data = _hook->action_data; \ + INSIST(_func != NULL); \ + if (_func(_qctx, _data, &_res)) { \ + result = _res; \ + goto cleanup; \ + } else { \ + _hook = ISC_LIST_NEXT(_hook, link); \ + } \ } \ - NS_PROCESS_HOOK(_tab, _id, _q, __VA_ARGS__); \ } while (false) -#define PROCESS_HOOK_VOID(_id, _qctx, ...) \ +/* + * Call the specified hook function in every configured module that + * implements that function. All modules are called; hook function return + * codes are ignored. This is intended for use with initialization and + * destruction calls which *must* run in every configured module. + * + * (This could be implemented as an inline void function, but is left as a + * macro for symmetry with PROCESS_HOOK above.) + */ +#define PROCESS_ALL_HOOKS(_id, _qctx) \ do { \ - ns_hooktable_t *_tab = ns__hook_table; \ - query_ctx_t *_q = (_qctx); \ - if (_q != NULL && \ - _q->view != NULL && \ - _q->view->hooktable != NULL) \ - { \ - _tab = _q->view->hooktable; \ + isc_result_t _res; \ + ns_hooktable_t *_tab = get_hooktab(_qctx); \ + ns_hook_t *_hook; \ + _hook = ISC_LIST_HEAD((*_tab)[_id]); \ + while (_hook != NULL) { \ + ns_hook_action_t _func = _hook->action; \ + void *_data = _hook->action_data; \ + INSIST(_func != NULL); \ + _func(_qctx, _data, &_res); \ + _hook = ISC_LIST_NEXT(_hook, link); \ } \ - NS_PROCESS_HOOK_VOID(_tab, _id, _q, __VA_ARGS__); \ } while (false) /* @@ -1908,6 +1951,7 @@ query_setorder(query_ctx_t *qctx, dns_name_t *name, dns_rdataset_t *rdataset) { static void query_additional(query_ctx_t *qctx, dns_rdataset_t *rdataset) { ns_client_t *client = qctx->client; + isc_result_t result; CTRACE(ISC_LOG_DEBUG(3), "query_additional"); @@ -1923,7 +1967,6 @@ query_additional(query_ctx_t *qctx, dns_rdataset_t *rdataset) { (client->query.gluedb != NULL) && dns_db_iszone(client->query.gluedb)) { - isc_result_t result; ns_dbversion_t *dbversion; dbversion = ns_client_findversion(client, client->query.gluedb); @@ -1938,7 +1981,7 @@ query_additional(query_ctx_t *qctx, dns_rdataset_t *rdataset) { } } -regular: + regular: /* * Add other additional data if needed. * We don't care if dns_rdataset_additionaldata() fails. @@ -4848,7 +4891,7 @@ qctx_init(ns_client_t *client, dns_fetchevent_t *event, qctx->answer_has_ns = false; qctx->authoritative = false; - PROCESS_HOOK_VOID(NS_QUERY_QCTX_INITIALIZED, qctx); + PROCESS_ALL_HOOKS(NS_QUERY_QCTX_INITIALIZED, qctx); } /*% @@ -4913,7 +4956,8 @@ qctx_freedata(query_ctx_t *qctx) { static void qctx_destroy(query_ctx_t *qctx) { - PROCESS_HOOK_VOID(NS_QUERY_QCTX_DESTROYED, qctx); + PROCESS_ALL_HOOKS(NS_QUERY_QCTX_DESTROYED, qctx); + dns_view_detach(&qctx->view); } @@ -4967,6 +5011,8 @@ query_setup(ns_client_t *client, dns_rdatatype_t qtype) { qctx_init(client, NULL, qtype, &qctx); query_trace(&qctx); + PROCESS_HOOK(NS_QUERY_SETUP, &qctx); + /* * If it's a SIG query, we'll iterate the node. */ @@ -4986,6 +5032,8 @@ query_setup(ns_client_t *client, dns_rdatatype_t qtype) { } result = ns__query_start(&qctx); + + cleanup: qctx_destroy(&qctx); return (result); } @@ -5263,6 +5311,9 @@ ns__query_start(query_ctx_t *qctx) { } return (query_lookup(qctx)); + + cleanup: + return (result); } /*% @@ -5386,6 +5437,9 @@ query_lookup(query_ctx_t *qctx) { } return (query_gotanswer(qctx, result)); + + cleanup: + return (result); } /* @@ -5889,6 +5943,9 @@ query_resume(query_ctx_t *qctx) { qctx->resuming = true; return (query_gotanswer(qctx, result)); + + cleanup: + return (result); } /*% @@ -6524,7 +6581,8 @@ query_usestale(query_ctx_t *qctx) { * result from the search. */ static isc_result_t -query_gotanswer(query_ctx_t *qctx, isc_result_t result) { +query_gotanswer(query_ctx_t *qctx, isc_result_t res) { + isc_result_t result = res; char errmsg[256]; CCTRACE(ISC_LOG_DEBUG(3), "query_gotanswer"); @@ -6624,6 +6682,9 @@ query_gotanswer(query_ctx_t *qctx, isc_result_t result) { QUERY_ERROR(qctx, result); return (ns_query_done(qctx)); } + + cleanup: + return (result); } static void @@ -6922,6 +6983,9 @@ query_respond_any(query_ctx_t *qctx) { query_addauth(qctx); return (ns_query_done(qctx)); + + cleanup: + return (result); } /* @@ -7139,6 +7203,9 @@ query_respond(query_ctx_t *qctx) { query_addauth(qctx); return (ns_query_done(qctx)); + + cleanup: + return (result); } static isc_result_t @@ -7555,6 +7622,9 @@ query_notfound(query_ctx_t *qctx) { } return (query_delegation(qctx)); + + cleanup: + return (result); } /*% @@ -7563,6 +7633,7 @@ query_notfound(query_ctx_t *qctx) { */ static isc_result_t query_prepare_delegation_response(query_ctx_t *qctx) { + isc_result_t result; dns_rdataset_t **sigrdatasetp = NULL; bool detach = false; @@ -7605,6 +7676,9 @@ query_prepare_delegation_response(query_ctx_t *qctx) { query_addds(qctx); return (ns_query_done(qctx)); + + cleanup: + return (result); } /*% @@ -7698,6 +7772,9 @@ query_zone_delegation(query_ctx_t *qctx) { } return (query_prepare_delegation_response(qctx)); + + cleanup: + return (result); } /*% @@ -7823,6 +7900,9 @@ query_delegation(query_ctx_t *qctx) { } return (query_prepare_delegation_response(qctx)); + + cleanup: + return (result); } /*% @@ -7963,7 +8043,9 @@ query_addds(query_ctx_t *qctx) { * Handle authoritative NOERROR/NODATA responses. */ static isc_result_t -query_nodata(query_ctx_t *qctx, isc_result_t result) { +query_nodata(query_ctx_t *qctx, isc_result_t res) { + isc_result_t result = res; + PROCESS_HOOK(NS_QUERY_NODATA_BEGIN, qctx); #ifdef dns64_bis_return_excluded_addresses @@ -8074,6 +8156,9 @@ query_nodata(query_ctx_t *qctx, isc_result_t result) { } return (ns_query_done(qctx)); + + cleanup: + return (result); } /*% @@ -8347,6 +8432,9 @@ query_nxdomain(query_ctx_t *qctx, bool empty_wild) { qctx->client->message->rcode = dns_rcode_nxdomain; return (ns_query_done(qctx)); + + cleanup: + return (result); } /* @@ -9282,6 +9370,9 @@ query_cname(query_ctx_t *qctx) { query_addauth(qctx); return (ns_query_done(qctx)); + + cleanup: + return (result); } /* @@ -9430,6 +9521,9 @@ query_dname(query_ctx_t *qctx) { query_addauth(qctx); return (ns_query_done(qctx)); + + cleanup: + return (result); } /*% @@ -9511,6 +9605,8 @@ query_addcname(query_ctx_t *qctx, dns_trust_t trust, dns_ttl_t ttl) { */ static isc_result_t query_prepresponse(query_ctx_t *qctx) { + isc_result_t result = ISC_R_SUCCESS; + PROCESS_HOOK(NS_QUERY_PREP_RESPONSE_BEGIN, qctx); if (WANTDNSSEC(qctx->client) && @@ -9524,9 +9620,12 @@ query_prepresponse(query_ctx_t *qctx) { if (qctx->type == dns_rdatatype_any) { return (query_respond_any(qctx)); - } else { - return (query_respond(qctx)); } + + return (query_respond(qctx)); + + cleanup: + return (result); } /*% @@ -10396,6 +10495,7 @@ query_glueanswer(query_ctx_t *qctx) { isc_result_t ns_query_done(query_ctx_t *qctx) { + isc_result_t result; const dns_namelist_t *secs = qctx->client->message->sections; CCTRACE(ISC_LOG_DEBUG(3), "ns_query_done"); @@ -10501,6 +10601,9 @@ ns_query_done(query_ctx_t *qctx) { ns_client_detach(&qctx->client); return (qctx->result); + + cleanup: + return (result); } static inline void diff --git a/lib/ns/tests/nstest.c b/lib/ns/tests/nstest.c index 355d175865..2532042e82 100644 --- a/lib/ns/tests/nstest.c +++ b/lib/ns/tests/nstest.c @@ -629,17 +629,17 @@ destroy_message: } /*% - * A hook callback which stores the query context pointed to by "hook_data" at - * "callback_data". Causes execution to be interrupted at hook insertion + * A hook action which stores the query context pointed to by "arg" at + * "data". Causes execution to be interrupted at hook insertion * point. */ static bool -extract_qctx(void *hook_data, void *callback_data, isc_result_t *resultp) { +extract_qctx(void *arg, void *data, isc_result_t *resultp) { query_ctx_t **qctxp; query_ctx_t *qctx; - REQUIRE(hook_data != NULL); - REQUIRE(callback_data != NULL); + REQUIRE(arg != NULL); + REQUIRE(data != NULL); REQUIRE(resultp != NULL); /* @@ -649,10 +649,10 @@ extract_qctx(void *hook_data, void *callback_data, isc_result_t *resultp) { */ qctx = isc_mem_get(mctx, sizeof(*qctx)); if (qctx != NULL) { - memmove(qctx, (query_ctx_t *)hook_data, sizeof(*qctx)); + memmove(qctx, (query_ctx_t *)arg, sizeof(*qctx)); } - qctxp = (query_ctx_t **)callback_data; + qctxp = (query_ctx_t **)data; /* * If memory allocation failed, the supplied pointer will simply be set * to NULL. We rely on the user of this hook to react properly. @@ -674,8 +674,8 @@ static isc_result_t create_qctx_for_client(ns_client_t *client, query_ctx_t **qctxp) { ns_hooktable_t *saved_hook_table, query_hooks; ns_hook_t hook = { - .callback = extract_qctx, - .callback_data = qctxp, + .action = extract_qctx, + .action_data = qctxp, }; REQUIRE(client != NULL); @@ -691,7 +691,7 @@ create_qctx_for_client(ns_client_t *client, query_ctx_t **qctxp) { */ ns_hooktable_init(&query_hooks); - ns_hook_add(&query_hooks, NS_QUERY_QCTX_INITIALIZED, &hook); + ns_hook_add(&query_hooks, NS_QUERY_SETUP, &hook); saved_hook_table = ns__hook_table; ns__hook_table = &query_hooks; @@ -801,11 +801,10 @@ ns_test_qctx_destroy(query_ctx_t **qctxp) { } bool -ns_test_hook_catch_call(void *hook_data, void *callback_data, - isc_result_t *resultp) +ns_test_hook_catch_call(void *arg, void *data, isc_result_t *resultp) { - UNUSED(hook_data); - UNUSED(callback_data); + UNUSED(arg); + UNUSED(data); *resultp = ISC_R_UNSET; diff --git a/lib/ns/tests/nstest.h b/lib/ns/tests/nstest.h index bc04487046..fc69befdc6 100644 --- a/lib/ns/tests/nstest.h +++ b/lib/ns/tests/nstest.h @@ -151,5 +151,4 @@ ns_test_qctx_destroy(query_ctx_t **qctxp); * A hook callback interrupting execution at given hook's insertion point. */ bool -ns_test_hook_catch_call(void *hook_data, void *callback_data, - isc_result_t *resultp); +ns_test_hook_catch_call(void *arg, void *data, isc_result_t *resultp); diff --git a/lib/ns/tests/query_test.c b/lib/ns/tests/query_test.c index f8f59a23b3..4b7002ae9c 100644 --- a/lib/ns/tests/query_test.c +++ b/lib/ns/tests/query_test.c @@ -85,7 +85,7 @@ run_sfcache_test(const ns__query_sfcache_test_params_t *test) { query_ctx_t *qctx = NULL; isc_result_t result; ns_hook_t hook = { - .callback = ns_test_hook_catch_call, + .action = ns_test_hook_catch_call, }; REQUIRE(test != NULL); @@ -282,7 +282,7 @@ run_start_test(const ns__query_start_test_params_t *test) { query_ctx_t *qctx = NULL; isc_result_t result; ns_hook_t hook = { - .callback = ns_test_hook_catch_call, + .action = ns_test_hook_catch_call, }; REQUIRE(test != NULL);