From 0d8c4044ab13e7d3b2dd0a7e25dbca34c08f2cdd Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 21 Oct 2025 15:08:47 +0200 Subject: [PATCH 01/12] cfg_obj_t file is now a refcounted string In order to reduce the lifecycle dependency of a `cfg_obj_t` on its parser, the `file` field needs its own reference count, so it isn't deleted when the parser is. It is now stored as a subsidiary `cfg_obj_t` object of type string. --- lib/isccfg/include/isccfg/grammar.h | 2 +- lib/isccfg/parser.c | 33 +++++++++++++++++------------ 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/lib/isccfg/include/isccfg/grammar.h b/lib/isccfg/include/isccfg/grammar.h index 6c3c1cb39b..ae15fd9ab2 100644 --- a/lib/isccfg/include/isccfg/grammar.h +++ b/lib/isccfg/include/isccfg/grammar.h @@ -198,7 +198,7 @@ struct cfg_obj { isccfg_duration_t duration; } value; isc_refcount_t references; /*%< reference counter */ - const char *file; + cfg_obj_t *file; /*%< refcounted string */ unsigned int line; cfg_parser_t *pctx; }; diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index 07bc4879fa..8a036f558b 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -3610,24 +3610,23 @@ have_current_file(cfg_parser_t *pctx) { return true; } -static char * +static cfg_obj_t * current_file(cfg_parser_t *pctx) { - static char none[] = "none"; cfg_listelt_t *elt; cfg_obj_t *fileobj; if (!have_current_file(pctx)) { - return none; + return NULL; } elt = ISC_LIST_TAIL(pctx->open_files->value.list); if (elt == NULL) { /* shouldn't be possible, but... */ - return none; + return NULL; } fileobj = elt->obj; INSIST(fileobj->type == &cfg_type_qstring); - return fileobj->value.string.base; + return fileobj; } static void @@ -3646,7 +3645,8 @@ parser_complain(cfg_parser_t *pctx, bool is_warning, unsigned int flags, where[0] = '\0'; if (have_current_file(pctx)) { - snprintf(where, sizeof(where), "%s:%u: ", current_file(pctx), + snprintf(where, sizeof(where), + "%s:%u: ", cfg_obj_asstring(current_file(pctx)), pctx->line); } else if (pctx->buf_name != NULL) { snprintf(where, sizeof(where), "%s: ", pctx->buf_name); @@ -3714,8 +3714,8 @@ cfg_obj_log(const cfg_obj_t *obj, int level, const char *fmt, ...) { va_end(ap); if (obj->file != NULL) { - isc_log_write(CAT, MOD, level, "%s:%u: %s", obj->file, - obj->line, msgbuf); + isc_log_write(CAT, MOD, level, "%s:%u: %s", + cfg_obj_asstring(obj->file), obj->line, msgbuf); } else { isc_log_write(CAT, MOD, level, "%s", msgbuf); } @@ -3725,7 +3725,7 @@ const char * cfg_obj_file(const cfg_obj_t *obj) { REQUIRE(obj != NULL); - return obj->file; + return cfg_obj_asstring(obj->file); } unsigned int @@ -3738,18 +3738,21 @@ cfg_obj_line(const cfg_obj_t *obj) { isc_result_t cfg_create_obj(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { cfg_obj_t *obj; + cfg_obj_t *file = NULL; REQUIRE(pctx != NULL); REQUIRE(type != NULL); REQUIRE(ret != NULL && *ret == NULL); obj = isc_mem_get(pctx->mctx, sizeof(cfg_obj_t)); - *obj = (cfg_obj_t){ .type = type, - .file = current_file(pctx), - .line = pctx->line, - .pctx = pctx }; + *obj = (cfg_obj_t){ .type = type, .line = pctx->line, .pctx = pctx }; isc_refcount_init(&obj->references, 1); + file = current_file(pctx); + if (file != NULL) { + cfg_obj_attach(file, &obj->file); + } + *ret = obj; return ISC_R_SUCCESS; @@ -3815,6 +3818,10 @@ cfg_obj_destroy(cfg_parser_t *pctx, cfg_obj_t **objp) { *objp = NULL; if (isc_refcount_decrement(&obj->references) == 1) { + if (obj->file != NULL) { + cfg_obj_destroy(obj->file->pctx, &obj->file); + } + obj->type->rep->free(pctx, obj); isc_refcount_destroy(&obj->references); isc_mem_put(pctx->mctx, obj, sizeof(cfg_obj_t)); From 7706f5acec93f90808d988152376ef563135be21 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 21 Oct 2025 15:41:55 +0200 Subject: [PATCH 02/12] remove parser context field from cfg_obj_t cfg_obj_t doesn't store a pointer to its a parser context anymore, and does not depend on the parser's lifecycle. Instead, it stores a reference to its own memory context (and in principle, each node could have different memory context). This also slightly simplifies the _destroy API as there is no need to pass a context through it anymore. --- bin/check/named-checkconf.c | 2 +- bin/delv/delv.c | 2 +- bin/dig/dighost.c | 2 +- bin/dnssec/dnssec-keygen.c | 2 +- bin/dnssec/dnssec-ksr.c | 2 +- bin/named/config.c | 2 +- bin/named/controlconf.c | 2 +- bin/named/main.c | 2 +- bin/named/server.c | 34 +-- bin/nsupdate/nsupdate.c | 2 +- bin/plugins/filter-a.c | 4 +- bin/plugins/filter-aaaa.c | 4 +- bin/plugins/synthrecord.c | 2 +- bin/rndc/rndc.c | 2 +- .../system/hooks/driver/test-syncplugin.c | 2 +- doc/misc/cfg_test.c | 2 +- lib/isccfg/include/isccfg/cfg.h | 11 +- lib/isccfg/include/isccfg/grammar.h | 15 +- lib/isccfg/namedconf.c | 40 ++-- lib/isccfg/parser.c | 195 +++++++++--------- tests/isccfg/duration_test.c | 2 +- tests/isccfg/parser_test.c | 6 +- 22 files changed, 182 insertions(+), 155 deletions(-) diff --git a/bin/check/named-checkconf.c b/bin/check/named-checkconf.c index a4da14d8e3..4f7bfcee56 100644 --- a/bin/check/named-checkconf.c +++ b/bin/check/named-checkconf.c @@ -745,7 +745,7 @@ main(int argc, char **argv) { cleanup: if (config != NULL) { - cfg_obj_destroy(parser, &config); + cfg_obj_destroy(&config); } if (parser != NULL) { diff --git a/bin/delv/delv.c b/bin/delv/delv.c index cfcc67fe5c..997c9fb73b 100644 --- a/bin/delv/delv.c +++ b/bin/delv/delv.c @@ -862,7 +862,7 @@ setup_dnsseckeys(dns_client_t *client, dns_view_t *toview) { cleanup: if (bindkeys != NULL) { - cfg_obj_destroy(parser, &bindkeys); + cfg_obj_destroy(&bindkeys); } if (parser != NULL) { cfg_parser_destroy(&parser); diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index c7d8016362..a79bf61f9d 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -1119,7 +1119,7 @@ read_confkey(void) { cleanup: if (pctx != NULL) { if (file != NULL) { - cfg_obj_destroy(pctx, &file); + cfg_obj_destroy(&file); } cfg_parser_destroy(&pctx); } diff --git a/bin/dnssec/dnssec-keygen.c b/bin/dnssec/dnssec-keygen.c index c7b4abe98a..04b8a8056f 100644 --- a/bin/dnssec/dnssec-keygen.c +++ b/bin/dnssec/dnssec-keygen.c @@ -1199,7 +1199,7 @@ main(int argc, char **argv) { } dns_kasp_detach(&kasp); - cfg_obj_destroy(parser, &config); + cfg_obj_destroy(&config); cfg_parser_destroy(&parser); } } else { diff --git a/bin/dnssec/dnssec-ksr.c b/bin/dnssec/dnssec-ksr.c index 86eb1362fc..073bbda1a9 100644 --- a/bin/dnssec/dnssec-ksr.c +++ b/bin/dnssec/dnssec-ksr.c @@ -187,7 +187,7 @@ getkasp(ksr_ctx_t *ksr, dns_kasp_t **kasp) { if (ISC_LIST_EMPTY(dns_kasp_keys(*kasp))) { fatal("dnssec-policy '%s' has no keys configured", ksr->policy); } - cfg_obj_destroy(parser, &config); + cfg_obj_destroy(&config); cfg_parser_destroy(&parser); } diff --git a/bin/named/config.c b/bin/named/config.c index ce0c17ac41..9268185444 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -457,7 +457,7 @@ named_config_parsefile(cfg_parser_t *parser, cfg_obj_t **conf) { cleanup: if (*conf) { - cfg_obj_destroy(parser, conf); + cfg_obj_destroy(conf); } out: diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c index 84003f86d6..26b0fb9197 100644 --- a/bin/named/controlconf.c +++ b/bin/named/controlconf.c @@ -861,7 +861,7 @@ cleanup: free_controlkey(keyid, mctx); } if (config != NULL) { - cfg_obj_destroy(pctx, &config); + cfg_obj_destroy(&config); } if (pctx != NULL) { cfg_parser_destroy(&pctx); diff --git a/bin/named/main.c b/bin/named/main.c index ef466ba7a8..3c12ba0d1d 100644 --- a/bin/named/main.c +++ b/bin/named/main.c @@ -618,7 +618,7 @@ printversion(bool verbose) { if (cfg_obj_isstring(obj)) { printf(" geoip-directory: %s\n", cfg_obj_asstring(obj)); } - cfg_obj_destroy(parser, &config); + cfg_obj_destroy(&config); cfg_parser_destroy(&parser); isc_mem_detach(&geoip_mctx); #endif /* HAVE_GEOIP2 */ diff --git a/bin/named/server.c b/bin/named/server.c index 82072750be..d8654be66d 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -2544,7 +2544,7 @@ cleanup: dns_zone_detach(&zone); } if (zoneconf != NULL) { - cfg_obj_destroy(cfg->add_parser, &zoneconf); + cfg_obj_destroy(&zoneconf); } if (dnsforwarders != NULL) { dns_forwarders_detach(&dnsforwarders); @@ -2783,7 +2783,7 @@ catz_reconfigure(dns_catz_entry_t *entry, void *arg1, void *arg2) { cleanup: if (zoneconf != NULL) { - cfg_obj_destroy(cfg->add_parser, &zoneconf); + cfg_obj_destroy(&zoneconf); } dns_zone_detach(&zone); @@ -7615,7 +7615,7 @@ data_to_cfg(dns_view_t *view, MDB_val *key, MDB_val *data, isc_buffer_t **text, cleanup: if (zoneconf != NULL) { - cfg_obj_destroy(named_g_addparser, &zoneconf); + cfg_obj_destroy(&zoneconf); } return result; @@ -7696,14 +7696,14 @@ for_all_newzone_cfgs(newzone_cfg_cb_t callback, cfg_obj_t *config, /* * Destroy the configuration object created in this iteration. */ - cfg_obj_destroy(named_g_addparser, &zconfigobj); + cfg_obj_destroy(&zconfigobj); } if (text != NULL) { isc_buffer_free(&text); } if (zconfigobj != NULL) { - cfg_obj_destroy(named_g_addparser, &zconfigobj); + cfg_obj_destroy(&zconfigobj); } mdb_cursor_close(cursor); @@ -7841,7 +7841,7 @@ cleanup: UNLOCK(&view->new_zone_lock); if (zoneconf != NULL) { - cfg_obj_destroy(named_g_addparser, &zoneconf); + cfg_obj_destroy(&zoneconf); } if (text != NULL) { isc_buffer_free(&text); @@ -9348,10 +9348,10 @@ load_configuration(named_server_t *server, bool first_time) { cleanup: if (bindkeys != NULL) { - cfg_obj_destroy(parser, &bindkeys); + cfg_obj_destroy(&bindkeys); } if (config != NULL) { - cfg_obj_destroy(parser, &config); + cfg_obj_destroy(&config); } cfg_parser_destroy(&parser); @@ -9594,7 +9594,7 @@ shutdown_server(void *arg) { cfg_aclconfctx_detach(&server->aclctx); } - cfg_obj_destroy(named_g_parser, &named_g_defaultconfig); + cfg_obj_destroy(&named_g_defaultconfig); cfg_parser_destroy(&named_g_parser); cfg_parser_destroy(&named_g_addparser); @@ -13029,7 +13029,7 @@ cleanup: } if (nzf_config != NULL) { - cfg_obj_destroy(named_g_addparser, &nzf_config); + cfg_obj_destroy(&nzf_config); } return result; @@ -13153,7 +13153,7 @@ newzone_parse(named_server_t *server, char *command, dns_view_t **viewp, cleanup: if (zoneconf != NULL) { - cfg_obj_destroy(named_g_addparser, &zoneconf); + cfg_obj_destroy(&zoneconf); } if (view != NULL) { dns_view_detach(&view); @@ -13197,7 +13197,7 @@ delete_zoneconf(dns_view_t *view, cfg_parser_t *pctx, const cfg_obj_t *config, e = UNCONST(elt); ISC_LIST_UNLINK(*list, e, link); - cfg_obj_destroy(pctx, &e->obj); + cfg_obj_destroy(&e->obj); isc_mem_put(pctx->mctx, e, sizeof(*e)); result = ISC_R_SUCCESS; break; @@ -13721,7 +13721,7 @@ cleanup: (void)putnull(text); } if (zoneconf != NULL) { - cfg_obj_destroy(named_g_addparser, &zoneconf); + cfg_obj_destroy(&zoneconf); } if (view != NULL) { dns_view_detach(&view); @@ -14212,7 +14212,7 @@ named_server_showzone(named_server_t *server, isc_lex_t *lex, cleanup: #ifdef HAVE_LMDB if (nzconfig != NULL) { - cfg_obj_destroy(named_g_addparser, &nzconfig); + cfg_obj_destroy(&nzconfig); } #endif /* HAVE_LMDB */ if (isc_buffer_usedlength(*text) > 0) { @@ -14232,16 +14232,16 @@ newzone_cfgctx_destroy(void **cfgp) { if (cfg->conf_parser != NULL) { if (cfg->config != NULL) { - cfg_obj_destroy(cfg->conf_parser, &cfg->config); + cfg_obj_destroy(&cfg->config); } if (cfg->vconfig != NULL) { - cfg_obj_destroy(cfg->conf_parser, &cfg->vconfig); + cfg_obj_destroy(&cfg->vconfig); } cfg_parser_destroy(&cfg->conf_parser); } if (cfg->add_parser != NULL) { if (cfg->nzf_config != NULL) { - cfg_obj_destroy(cfg->add_parser, &cfg->nzf_config); + cfg_obj_destroy(&cfg->nzf_config); } cfg_parser_destroy(&cfg->add_parser); } diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index 8f8b47af15..c9d9ce0adf 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -604,7 +604,7 @@ read_sessionkey(isc_mem_t *mctx) { cleanup: if (pctx != NULL) { if (sessionkey != NULL) { - cfg_obj_destroy(pctx, &sessionkey); + cfg_obj_destroy(&sessionkey); } cfg_parser_destroy(&pctx); } diff --git a/bin/plugins/filter-a.c b/bin/plugins/filter-a.c index e9c314b008..8b99c6c4bc 100644 --- a/bin/plugins/filter-a.c +++ b/bin/plugins/filter-a.c @@ -300,7 +300,7 @@ parse_parameters(filter_instance_t *inst, const char *parameters, cleanup: if (param_obj != NULL) { - cfg_obj_destroy(parser, ¶m_obj); + cfg_obj_destroy(¶m_obj); } if (parser != NULL) { cfg_parser_destroy(&parser); @@ -383,7 +383,7 @@ plugin_check(const char *parameters, const void *cfg, const char *cfg_file, cleanup: if (param_obj != NULL) { - cfg_obj_destroy(parser, ¶m_obj); + cfg_obj_destroy(¶m_obj); } if (parser != NULL) { cfg_parser_destroy(&parser); diff --git a/bin/plugins/filter-aaaa.c b/bin/plugins/filter-aaaa.c index 84dd0acd62..b9a2b52d54 100644 --- a/bin/plugins/filter-aaaa.c +++ b/bin/plugins/filter-aaaa.c @@ -303,7 +303,7 @@ parse_parameters(filter_instance_t *inst, const char *parameters, cleanup: if (param_obj != NULL) { - cfg_obj_destroy(parser, ¶m_obj); + cfg_obj_destroy(¶m_obj); } if (parser != NULL) { cfg_parser_destroy(&parser); @@ -387,7 +387,7 @@ plugin_check(const char *parameters, const void *cfg, const char *cfg_file, cleanup: if (param_obj != NULL) { - cfg_obj_destroy(parser, ¶m_obj); + cfg_obj_destroy(¶m_obj); } if (parser != NULL) { cfg_parser_destroy(&parser); diff --git a/bin/plugins/synthrecord.c b/bin/plugins/synthrecord.c index b5582bad49..e963cdad5b 100644 --- a/bin/plugins/synthrecord.c +++ b/bin/plugins/synthrecord.c @@ -593,7 +593,7 @@ synthrecord_parseconfig(synthrecord_t *inst, const char *parameters, cleanup: if (synthrecordcfg != NULL) { - cfg_obj_destroy(parser, &synthrecordcfg); + cfg_obj_destroy(&synthrecordcfg); } if (parser != NULL) { diff --git a/bin/rndc/rndc.c b/bin/rndc/rndc.c index d191fd97d1..6aa1e4e67b 100644 --- a/bin/rndc/rndc.c +++ b/bin/rndc/rndc.c @@ -999,7 +999,7 @@ main(int argc, char **argv) { isccc_ccmsg_invalidate(&rndc_ccmsg); - cfg_obj_destroy(pctx, &config); + cfg_obj_destroy(&config); cfg_parser_destroy(&pctx); isc_mem_put(isc_g_mctx, args, argslen); diff --git a/bin/tests/system/hooks/driver/test-syncplugin.c b/bin/tests/system/hooks/driver/test-syncplugin.c index 556a91d065..86bea21cf4 100644 --- a/bin/tests/system/hooks/driver/test-syncplugin.c +++ b/bin/tests/system/hooks/driver/test-syncplugin.c @@ -217,7 +217,7 @@ cleanup: } if (syncplugincfg != NULL) { - cfg_obj_destroy(parser, &syncplugincfg); + cfg_obj_destroy(&syncplugincfg); } if (parser != NULL) { diff --git a/doc/misc/cfg_test.c b/doc/misc/cfg_test.c index d039bf07cf..db596e3df8 100644 --- a/doc/misc/cfg_test.c +++ b/doc/misc/cfg_test.c @@ -146,7 +146,7 @@ main(int argc, char **argv) { cfg_print(cfg, output, NULL); - cfg_obj_destroy(pctx, &cfg); + cfg_obj_destroy(&cfg); cfg_parser_destroy(&pctx); } diff --git a/lib/isccfg/include/isccfg/cfg.h b/lib/isccfg/include/isccfg/cfg.h index 34ca10b793..77576fcf2b 100644 --- a/lib/isccfg/include/isccfg/cfg.h +++ b/lib/isccfg/include/isccfg/cfg.h @@ -191,6 +191,13 @@ cfg_parser_destroy(cfg_parser_t **pctxp); * more references. */ +cfg_obj_t * +cfg_parser_currentfile(cfg_parser_t *pctx); +/*%< + * Returns the current file of a parser (as an cfg_obj_t qstring). NULL is non + * existent. + */ + bool cfg_obj_isvoid(const cfg_obj_t *obj); /*%< @@ -550,14 +557,14 @@ cfg_obj_attach(cfg_obj_t *src, cfg_obj_t **dest); */ void -cfg_obj_destroy(cfg_parser_t *pctx, cfg_obj_t **obj); +cfg_obj_destroy(cfg_obj_t **obj); /*%< * Delete a reference to a configuration object; destroy the object if * there are no more references. * * Require: * \li '*obj' is a valid cfg_obj_t. - * \li 'pctx' is a valid cfg_parser_t. + * \li 'mctx' is a valid isc_mem_t. */ void diff --git a/lib/isccfg/include/isccfg/grammar.h b/lib/isccfg/include/isccfg/grammar.h index ae15fd9ab2..c06fedefb2 100644 --- a/lib/isccfg/include/isccfg/grammar.h +++ b/lib/isccfg/include/isccfg/grammar.h @@ -102,7 +102,7 @@ typedef isc_result_t (*cfg_parsefunc_t)(cfg_parser_t *, const cfg_type_t *type, cfg_obj_t **); typedef void (*cfg_printfunc_t)(cfg_printer_t *, const cfg_obj_t *); typedef void (*cfg_docfunc_t)(cfg_printer_t *, const cfg_type_t *); -typedef void (*cfg_freefunc_t)(cfg_parser_t *, cfg_obj_t *); +typedef void (*cfg_freefunc_t)(cfg_obj_t *); /* * Structure definitions @@ -180,6 +180,10 @@ struct cfg_rep { */ struct cfg_obj { + unsigned int magic; + isc_mem_t *mctx; + isc_refcount_t references; + const cfg_type_t *type; union { uint32_t uint32; @@ -197,10 +201,8 @@ struct cfg_obj { cfg_netprefix_t netprefix; isccfg_duration_t duration; } value; - isc_refcount_t references; /*%< reference counter */ - cfg_obj_t *file; /*%< refcounted string */ - unsigned int line; - cfg_parser_t *pctx; + cfg_obj_t *file; /*%< separate string with its own refcount */ + unsigned int line; }; /*% A list element. */ @@ -354,7 +356,8 @@ cfg_ungettoken(cfg_parser_t *pctx); #define CFG_LEXOPT_QSTRING (ISC_LEXOPT_QSTRING | ISC_LEXOPT_QSTRINGMULTILINE) isc_result_t -cfg_create_obj(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **objp); +cfg_create_obj(isc_mem_t *mctx, cfg_obj_t *file, size_t line, + const cfg_type_t *type, cfg_obj_t **ret); void cfg_print_rawuint(cfg_printer_t *pctx, unsigned int u); diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c index 4b022cefe2..46e0c54d6d 100644 --- a/lib/isccfg/namedconf.c +++ b/lib/isccfg/namedconf.c @@ -42,10 +42,10 @@ } while (0) /*% Clean up a configuration object if non-NULL. */ -#define CLEANUP_OBJ(obj) \ - do { \ - if ((obj) != NULL) \ - cfg_obj_destroy(pctx, &(obj)); \ +#define CLEANUP_OBJ(obj) \ + do { \ + if ((obj) != NULL) \ + cfg_obj_destroy(&(obj)); \ } while (0) /*% @@ -414,7 +414,8 @@ parse_updatepolicy(cfg_parser_t *pctx, const cfg_type_t *type, strcasecmp(TOKEN_STRING(pctx), "local") == 0) { cfg_obj_t *obj = NULL; - CHECK(cfg_create_obj(pctx, &cfg_type_ustring, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_ustring, &obj)); obj->value.string.length = strlen("local"); obj->value.string.base = isc_mem_get(pctx->mctx, obj->value.string.length + 1); @@ -972,7 +973,8 @@ parse_qstringornone(cfg_parser_t *pctx, const cfg_type_t *type, if (pctx->token.type == isc_tokentype_string && strcasecmp(TOKEN_STRING(pctx), "none") == 0) { - return cfg_create_obj(pctx, &cfg_type_none, ret); + return cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_none, ret); } cfg_ungettoken(pctx); return cfg_parse_qstring(pctx, type, ret); @@ -1014,7 +1016,8 @@ parse_boolorauto(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { if (pctx->token.type == isc_tokentype_string && strcasecmp(TOKEN_STRING(pctx), "auto") == 0) { - return cfg_create_obj(pctx, &cfg_type_auto, ret); + return cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_auto, ret); } cfg_ungettoken(pctx); return cfg_parse_boolean(pctx, type, ret); @@ -1068,12 +1071,15 @@ parse_serverid(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { if (pctx->token.type == isc_tokentype_string && strcasecmp(TOKEN_STRING(pctx), "none") == 0) { - return cfg_create_obj(pctx, &cfg_type_none, ret); + return cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_none, ret); } if (pctx->token.type == isc_tokentype_string && strcasecmp(TOKEN_STRING(pctx), "hostname") == 0) { - result = cfg_create_obj(pctx, &cfg_type_hostname, ret); + result = cfg_create_obj(pctx->mctx, + cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_hostname, ret); if (result == ISC_R_SUCCESS) { (*ret)->value.boolean = true; } @@ -2814,7 +2820,8 @@ parse_sizeval(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { } CHECK(parse_unitstring(TOKEN_STRING(pctx), &val)); - CHECK(cfg_create_obj(pctx, &cfg_type_uint64, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_uint64, &obj)); obj->value.uint64 = val; *ret = obj; return ISC_R_SUCCESS; @@ -2845,13 +2852,15 @@ parse_sizeval_percent(cfg_parser_t *pctx, const cfg_type_t *type, percent = strtoull(TOKEN_STRING(pctx), &endp, 10); if (*endp == '%' && *(endp + 1) == 0) { - CHECK(cfg_create_obj(pctx, &cfg_type_percentage, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_percentage, &obj)); obj->value.uint32 = (uint32_t)percent; *ret = obj; return ISC_R_SUCCESS; } else { CHECK(parse_unitstring(TOKEN_STRING(pctx), &val)); - CHECK(cfg_create_obj(pctx, &cfg_type_uint64, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_uint64, &obj)); obj->value.uint64 = val; *ret = obj; return ISC_R_SUCCESS; @@ -3278,7 +3287,8 @@ parse_querysource(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { strcasecmp(TOKEN_STRING(pctx), "none") == 0) { CHECK(cfg_gettoken(pctx, 0)); - CHECK(cfg_create_obj(pctx, &cfg_type_none, ret)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_none, ret)); } else { CHECK(cfg_parse_sockaddr_generic(pctx, &cfg_type_querysource, type, ret)); @@ -3479,7 +3489,9 @@ parse_logseverity(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { * This makes little sense, but we support it for * compatibility with BIND 8. */ - CHECK(cfg_create_obj(pctx, &cfg_type_uint32, ret)); + CHECK(cfg_create_obj( + pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_uint32, ret)); (*ret)->value.uint32 = 1; } (*ret)->type = &cfg_type_debuglevel; /* XXX kludge */ diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index 8a036f558b..9042493bb0 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -88,18 +88,21 @@ } while (0) /* Clean up a configuration object if non-NULL. */ -#define CLEANUP_OBJ(obj) \ - do { \ - if ((obj) != NULL) \ - cfg_obj_destroy(pctx, &(obj)); \ +#define CLEANUP_OBJ(obj) \ + do { \ + if ((obj) != NULL) \ + cfg_obj_destroy(&(obj)); \ } while (0) +/* cfg_obj_t magic number */ +#define CFGOBJ_MAGIC ISC_MAGIC('c', 'f', 'g', 'o') + /* * Forward declarations of static functions. */ static void -free_tuple(cfg_parser_t *pctx, cfg_obj_t *obj); +free_tuple(cfg_obj_t *obj); static isc_result_t parse_list(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret); @@ -108,7 +111,7 @@ static void print_list(cfg_printer_t *pctx, const cfg_obj_t *obj); static void -free_list(cfg_parser_t *pctx, cfg_obj_t *obj); +free_list(cfg_obj_t *obj); static isc_result_t create_listelt(cfg_parser_t *pctx, cfg_listelt_t **eltp); @@ -118,23 +121,23 @@ create_string(cfg_parser_t *pctx, const char *contents, const cfg_type_t *type, cfg_obj_t **ret); static void -free_string(cfg_parser_t *pctx, cfg_obj_t *obj); +free_string(cfg_obj_t *obj); static void -free_sockaddrtls(cfg_parser_t *pctx, cfg_obj_t *obj); +free_sockaddrtls(cfg_obj_t *obj); static isc_result_t create_map(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **objp); static void -free_map(cfg_parser_t *pctx, cfg_obj_t *obj); +free_map(cfg_obj_t *obj); static isc_result_t parse_symtab_elt(cfg_parser_t *pctx, const char *name, cfg_type_t *elttype, isc_symtab_t *symtab, bool callback); static void -free_noop(cfg_parser_t *pctx, cfg_obj_t *obj); +free_noop(cfg_obj_t *obj); static isc_result_t cfg_getstringtoken(cfg_parser_t *pctx); @@ -236,6 +239,25 @@ print_close(cfg_printer_t *pctx) { cfg_print_cstr(pctx, "}"); } +cfg_obj_t * +cfg_parser_currentfile(cfg_parser_t *pctx) { + cfg_listelt_t *elt = NULL; + cfg_obj_t *fileobj = NULL; + + if (pctx->open_files == NULL) { + return NULL; + } + + elt = ISC_LIST_TAIL(pctx->open_files->value.list); + if (elt == NULL) { + return NULL; + } + + fileobj = elt->obj; + INSIST(fileobj->type == &cfg_type_qstring); + return fileobj; +} + isc_result_t cfg_parse_obj(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { isc_result_t result; @@ -299,7 +321,8 @@ cfg_create_tuple(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { nfields++; } - CHECK(cfg_create_obj(pctx, type, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, type, &obj)); obj->value.tuple = isc_mem_cget(pctx->mctx, nfields, sizeof(cfg_obj_t *)); for (f = fields, i = 0; f->name != NULL; f++, i++) { @@ -386,7 +409,7 @@ cfg_doc_tuple(cfg_printer_t *pctx, const cfg_type_t *type) { } static void -free_tuple(cfg_parser_t *pctx, cfg_obj_t *obj) { +free_tuple(cfg_obj_t *obj) { unsigned int i; const cfg_tuplefielddef_t *fields = obj->type->of; const cfg_tuplefielddef_t *f; @@ -400,8 +423,7 @@ free_tuple(cfg_parser_t *pctx, cfg_obj_t *obj) { CLEANUP_OBJ(obj->value.tuple[i]); nfields++; } - isc_mem_cput(pctx->mctx, obj->value.tuple, nfields, - sizeof(cfg_obj_t *)); + isc_mem_cput(obj->mctx, obj->value.tuple, nfields, sizeof(cfg_obj_t *)); } bool @@ -746,7 +768,8 @@ cfg_parse_void(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { UNUSED(type); - return cfg_create_obj(pctx, &cfg_type_void, ret); + return cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_void, ret); } void @@ -804,7 +827,8 @@ cfg_parse_percentage(cfg_parser_t *pctx, const cfg_type_t *type, return ISC_R_UNEXPECTEDTOKEN; } - CHECK(cfg_create_obj(pctx, &cfg_type_percentage, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_percentage, &obj)); obj->value.uint32 = (uint32_t)percent; *ret = obj; @@ -877,7 +901,8 @@ cfg_parse_fixedpoint(cfg_parser_t *pctx, const cfg_type_t *type, return ISC_R_UNEXPECTEDTOKEN; } - CHECK(cfg_create_obj(pctx, &cfg_type_fixedpoint, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_fixedpoint, &obj)); obj->value.uint32 = strtoul(p, NULL, 10) * 100; switch (n3) { @@ -943,7 +968,8 @@ cfg_parse_uint32(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { return ISC_R_UNEXPECTEDTOKEN; } - CHECK(cfg_create_obj(pctx, &cfg_type_uint32, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_uint32, &obj)); obj->value.uint32 = pctx->token.value.as_ulong; *ret = obj; @@ -1164,7 +1190,8 @@ parse_duration(cfg_parser_t *pctx, cfg_obj_t **ret) { goto cleanup; } - CHECK(cfg_create_obj(pctx, &cfg_type_duration, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_duration, &obj)); obj->value.duration = duration; *ret = obj; @@ -1219,7 +1246,8 @@ cfg_parse_duration_or_unlimited(cfg_parser_t *pctx, const cfg_type_t *type, duration.iso8601 = false; duration.unlimited = true; - CHECK(cfg_create_obj(pctx, &cfg_type_duration, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_duration, &obj)); obj->value.duration = duration; *ret = obj; return ISC_R_SUCCESS; @@ -1271,7 +1299,8 @@ create_string(cfg_parser_t *pctx, const char *contents, const cfg_type_t *type, cfg_obj_t *obj = NULL; int len; - CHECK(cfg_create_obj(pctx, type, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, type, &obj)); len = strlen(contents); obj->value.string.length = len; obj->value.string.base = isc_mem_get(pctx->mctx, len + 1); @@ -1543,16 +1572,16 @@ print_sstring(cfg_printer_t *pctx, const cfg_obj_t *obj) { } static void -free_string(cfg_parser_t *pctx, cfg_obj_t *obj) { - isc_mem_put(pctx->mctx, obj->value.string.base, +free_string(cfg_obj_t *obj) { + isc_mem_put(obj->mctx, obj->value.string.base, obj->value.string.length + 1); } static void -free_sockaddrtls(cfg_parser_t *pctx, cfg_obj_t *obj) { +free_sockaddrtls(cfg_obj_t *obj) { if (obj->value.sockaddrtls.tls.base != NULL) { INSIST(obj->value.sockaddrtls.tls.length != 0); - isc_mem_put(pctx->mctx, obj->value.sockaddrtls.tls.base, + isc_mem_put(obj->mctx, obj->value.sockaddrtls.tls.base, obj->value.sockaddrtls.tls.length + 1); } } @@ -1881,7 +1910,8 @@ cfg_parse_boolean(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { goto bad_boolean; } - CHECK(cfg_create_obj(pctx, &cfg_type_boolean, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_boolean, &obj)); obj->value.boolean = value; *ret = obj; return result; @@ -1922,7 +1952,8 @@ cfg_create_list(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **obj) { REQUIRE(type != NULL); REQUIRE(obj != NULL && *obj == NULL); - CHECK(cfg_create_obj(pctx, type, obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, type, obj)); ISC_LIST_INIT((*obj)->value.list); cleanup: return result; @@ -1940,17 +1971,17 @@ create_listelt(cfg_parser_t *pctx, cfg_listelt_t **eltp) { } static void -free_listelt(cfg_parser_t *pctx, cfg_listelt_t *elt) { +free_listelt(isc_mem_t *mctx, cfg_listelt_t *elt) { if (elt->obj != NULL) { - cfg_obj_destroy(pctx, &elt->obj); + cfg_obj_destroy(&elt->obj); } - isc_mem_put(pctx->mctx, elt, sizeof(*elt)); + isc_mem_put(mctx, elt, sizeof(*elt)); } static void -free_list(cfg_parser_t *pctx, cfg_obj_t *obj) { +free_list(cfg_obj_t *obj) { ISC_LIST_FOREACH(obj->value.list, elt, link) { - free_listelt(pctx, elt); + free_listelt(obj->mctx, elt); } } @@ -2012,7 +2043,7 @@ parse_list(cfg_parser_t *pctx, const cfg_type_t *listtype, cfg_obj_t **ret) { cleanup: if (elt != NULL) { - free_listelt(pctx, elt); + free_listelt(pctx->mctx, elt); } CLEANUP_OBJ(listobj); return result; @@ -2274,7 +2305,7 @@ cfg_parse_mapbody(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { CHECK(parser_openfile(pctx, g.gl_pathv[i])); } - cfg_obj_destroy(pctx, &includename); + cfg_obj_destroy(&includename); globfree(&g); goto redo; @@ -2302,7 +2333,7 @@ cfg_parse_mapbody(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { */ CHECK(cfg_parse_obj(pctx, &cfg_type_unsupported, &eltobj)); - cfg_obj_destroy(pctx, &eltobj); + cfg_obj_destroy(&eltobj); CHECK(parse_semicolon(pctx)); continue; } @@ -2786,7 +2817,8 @@ parse_token(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { UNUSED(type); - CHECK(cfg_create_obj(pctx, &cfg_type_token, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_token, &obj)); CHECK(cfg_gettoken(pctx, CFG_LEXOPT_QSTRING)); if (pctx->token.type == isc_tokentype_eof) { cfg_ungettoken(pctx); @@ -3056,7 +3088,8 @@ parse_netaddr(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { isc_netaddr_t netaddr; unsigned int flags = *(const unsigned int *)type->of; - CHECK(cfg_create_obj(pctx, type, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, type, &obj)); CHECK(cfg_parse_rawaddr(pctx, flags, &netaddr)); isc_sockaddr_fromnetaddr(&obj->value.sockaddr, &netaddr, 0); *ret = obj; @@ -3185,7 +3218,8 @@ cfg_parse_netprefix(cfg_parser_t *pctx, const cfg_type_t *type, } prefixlen = addrlen; } - CHECK(cfg_create_obj(pctx, &cfg_type_netprefix, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_netprefix, &obj)); obj->value.netprefix.address = netaddr; obj->value.netprefix.prefixlen = prefixlen; *ret = obj; @@ -3313,7 +3347,8 @@ parse_sockaddrsub(cfg_parser_t *pctx, const cfg_type_t *type, int flags, goto cleanup; } - CHECK(cfg_create_obj(pctx, type, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, type, &obj)); if (have_tls == 1) { obj->value.sockaddrtls.tls = tls; } @@ -3595,40 +3630,6 @@ cfg_parser_warning(cfg_parser_t *pctx, unsigned int flags, const char *fmt, #define MAX_LOG_TOKEN 30 /* How much of a token to quote in log messages. */ -static bool -have_current_file(cfg_parser_t *pctx) { - cfg_listelt_t *elt; - if (pctx->open_files == NULL) { - return false; - } - - elt = ISC_LIST_TAIL(pctx->open_files->value.list); - if (elt == NULL) { - return false; - } - - return true; -} - -static cfg_obj_t * -current_file(cfg_parser_t *pctx) { - cfg_listelt_t *elt; - cfg_obj_t *fileobj; - - if (!have_current_file(pctx)) { - return NULL; - } - - elt = ISC_LIST_TAIL(pctx->open_files->value.list); - if (elt == NULL) { /* shouldn't be possible, but... */ - return NULL; - } - - fileobj = elt->obj; - INSIST(fileobj->type == &cfg_type_qstring); - return fileobj; -} - static void parser_complain(cfg_parser_t *pctx, bool is_warning, unsigned int flags, const char *format, va_list args) { @@ -3637,6 +3638,7 @@ parser_complain(cfg_parser_t *pctx, bool is_warning, unsigned int flags, static char message[2048]; int level = ISC_LOG_ERROR; const char *prep = ""; + const cfg_obj_t *file = NULL; size_t len; if (is_warning) { @@ -3644,10 +3646,10 @@ parser_complain(cfg_parser_t *pctx, bool is_warning, unsigned int flags, } where[0] = '\0'; - if (have_current_file(pctx)) { + file = cfg_parser_currentfile(pctx); + if (file != NULL) { snprintf(where, sizeof(where), - "%s:%u: ", cfg_obj_asstring(current_file(pctx)), - pctx->line); + "%s:%u: ", cfg_obj_asstring(file), pctx->line); } else if (pctx->buf_name != NULL) { snprintf(where, sizeof(where), "%s: ", pctx->buf_name); } @@ -3736,19 +3738,19 @@ cfg_obj_line(const cfg_obj_t *obj) { } isc_result_t -cfg_create_obj(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { +cfg_create_obj(isc_mem_t *mctx, cfg_obj_t *file, size_t line, + const cfg_type_t *type, cfg_obj_t **ret) { cfg_obj_t *obj; - cfg_obj_t *file = NULL; - REQUIRE(pctx != NULL); + REQUIRE(mctx != NULL); REQUIRE(type != NULL); REQUIRE(ret != NULL && *ret == NULL); - obj = isc_mem_get(pctx->mctx, sizeof(cfg_obj_t)); - *obj = (cfg_obj_t){ .type = type, .line = pctx->line, .pctx = pctx }; - isc_refcount_init(&obj->references, 1); + obj = isc_mem_get(mctx, sizeof(cfg_obj_t)); + *obj = (cfg_obj_t){ .magic = CFGOBJ_MAGIC, .type = type, .line = line }; - file = current_file(pctx); + isc_refcount_init(&obj->references, 1); + isc_mem_attach(mctx, &obj->mctx); if (file != NULL) { cfg_obj_attach(file, &obj->file); } @@ -3762,12 +3764,12 @@ static void map_symtabitem_destroy(char *key, unsigned int type, isc_symvalue_t symval, void *userarg) { cfg_obj_t *obj = symval.as_pointer; - cfg_parser_t *pctx = (cfg_parser_t *)userarg; UNUSED(key); UNUSED(type); + UNUSED(userarg); - cfg_obj_destroy(pctx, &obj); + cfg_obj_destroy(&obj); } static isc_result_t @@ -3776,7 +3778,8 @@ create_map(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { isc_symtab_t *symtab = NULL; cfg_obj_t *obj = NULL; - CHECK(cfg_create_obj(pctx, type, &obj)); + CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, type, &obj)); isc_symtab_create(pctx->mctx, map_symtabitem_destroy, pctx, false, &symtab); obj->value.map.symtab = symtab; @@ -3793,7 +3796,7 @@ cleanup: } static void -free_map(cfg_parser_t *pctx, cfg_obj_t *obj) { +free_map(cfg_obj_t *obj) { CLEANUP_OBJ(obj->value.map.id); isc_symtab_destroy(&obj->value.map.symtab); } @@ -3810,21 +3813,24 @@ cfg_obj_istype(const cfg_obj_t *obj, const cfg_type_t *type) { * Destroy 'obj', a configuration object created in 'pctx'. */ void -cfg_obj_destroy(cfg_parser_t *pctx, cfg_obj_t **objp) { +cfg_obj_destroy(cfg_obj_t **objp) { REQUIRE(objp != NULL && *objp != NULL); - REQUIRE(pctx != NULL); + REQUIRE((*objp)->magic == CFGOBJ_MAGIC); cfg_obj_t *obj = *objp; *objp = NULL; if (isc_refcount_decrement(&obj->references) == 1) { + obj->magic = 0; + if (obj->file != NULL) { - cfg_obj_destroy(obj->file->pctx, &obj->file); + cfg_obj_destroy(&obj->file); } - obj->type->rep->free(pctx, obj); + obj->type->rep->free(obj); + isc_refcount_destroy(&obj->references); - isc_mem_put(pctx->mctx, obj, sizeof(cfg_obj_t)); + isc_mem_putanddetach(&obj->mctx, obj, sizeof(cfg_obj_t)); } } @@ -3838,8 +3844,7 @@ cfg_obj_attach(cfg_obj_t *src, cfg_obj_t **dest) { } static void -free_noop(cfg_parser_t *pctx, cfg_obj_t *obj) { - UNUSED(pctx); +free_noop(cfg_obj_t *obj) { UNUSED(obj); } @@ -3943,7 +3948,7 @@ breakout: cleanup: if (elt != NULL) { - free_listelt(pctx, elt); + free_listelt(pctx->mctx, elt); } CLEANUP_OBJ(destobj); diff --git a/tests/isccfg/duration_test.c b/tests/isccfg/duration_test.c index 8d39ef5939..a9524a0c4a 100644 --- a/tests/isccfg/duration_test.c +++ b/tests/isccfg/duration_test.c @@ -192,7 +192,7 @@ ISC_RUN_TEST_IMPL(duration) { assert_int_equal(cmp, 0); } - cfg_obj_destroy(p1, &c1); + cfg_obj_destroy(&c1); cfg_parser_destroy(&p1); } } diff --git a/tests/isccfg/parser_test.c b/tests/isccfg/parser_test.c index da95ddded0..7efc320dae 100644 --- a/tests/isccfg/parser_test.c +++ b/tests/isccfg/parser_test.c @@ -102,7 +102,7 @@ ISC_RUN_TEST_IMPL(addzoneconf) { strlcat(buf, ";", sizeof(buf)); assert_string_equal(tests[i], buf); - cfg_obj_destroy(p, &conf); + cfg_obj_destroy(&conf); cfg_parser_reset(p); } @@ -141,8 +141,8 @@ ISC_RUN_TEST_IMPL(parse_buffer) { assert_int_equal(result, ISC_R_SUCCESS); assert_int_equal(p2->line, 104); - cfg_obj_destroy(p1, &c1); - cfg_obj_destroy(p2, &c2); + cfg_obj_destroy(&c1); + cfg_obj_destroy(&c2); cfg_parser_destroy(&p1); cfg_parser_destroy(&p2); From a72b8a1a60c0b3f31f00a710b06db5560794f3a6 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 21 Oct 2025 16:29:34 +0200 Subject: [PATCH 03/12] remove global named defaults parser Remove the global named defaults parser. Instead, a parser is created during the execution time of named_config_parsedefaults(). This simplifies the API (no parser to pass around) and the life-cycle of the default configuration tree (it doesn't depends on a parser instance). --- bin/named/config.c | 20 +++++++++++++++----- bin/named/include/named/config.h | 2 +- bin/named/include/named/globals.h | 1 - bin/named/main.c | 5 +---- bin/named/server.c | 7 +------ 5 files changed, 18 insertions(+), 17 deletions(-) diff --git a/bin/named/config.c b/bin/named/config.c index 9268185444..d08e94c959 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -361,15 +361,25 @@ remote-servers " DEFAULT_IANA_ROOT_ZONE_PRIMARIES " {\n\ "; isc_result_t -named_config_parsedefaults(cfg_parser_t *parser, cfg_obj_t **conf) { +named_config_parsedefaults(cfg_obj_t **conf) { isc_buffer_t b; + cfg_parser_t *parser = NULL; + isc_result_t result; + + result = cfg_parser_create(isc_g_mctx, &parser); + if (result != ISC_R_SUCCESS) { + return result; + } isc_buffer_init(&b, defaultconf, sizeof(defaultconf) - 1); isc_buffer_add(&b, sizeof(defaultconf) - 1); - return cfg_parse_buffer(parser, &b, __FILE__, 0, &cfg_type_namedconf, - CFG_PCTX_NODEPRECATED | CFG_PCTX_NOOBSOLETE | - CFG_PCTX_NOEXPERIMENTAL, - conf); + result = cfg_parse_buffer(parser, &b, __FILE__, 0, &cfg_type_namedconf, + CFG_PCTX_NODEPRECATED | CFG_PCTX_NOOBSOLETE | + CFG_PCTX_NOEXPERIMENTAL, + conf); + + cfg_parser_destroy(&parser); + return result; } /* diff --git a/bin/named/include/named/config.h b/bin/named/include/named/config.h index 744f583423..6b275727d4 100644 --- a/bin/named/include/named/config.h +++ b/bin/named/include/named/config.h @@ -25,7 +25,7 @@ #define DEFAULT_IANA_ROOT_ZONE_PRIMARIES "_default_iana_root_zone_primaries" isc_result_t -named_config_parsedefaults(cfg_parser_t *parser, cfg_obj_t **conf); +named_config_parsedefaults(cfg_obj_t **conf); isc_result_t named_config_parsefile(cfg_parser_t *parser, cfg_obj_t **conf); diff --git a/bin/named/include/named/globals.h b/bin/named/include/named/globals.h index fa5fd5581c..c087fe60df 100644 --- a/bin/named/include/named/globals.h +++ b/bin/named/include/named/globals.h @@ -61,7 +61,6 @@ EXTERN bool named_g_run_done INIT(false); * for really short timers, another for client timers, and one * for zone timers. */ -EXTERN cfg_parser_t *named_g_parser INIT(NULL); EXTERN cfg_parser_t *named_g_addparser INIT(NULL); EXTERN const char *named_g_version INIT(PACKAGE_VERSION); EXTERN const char *named_g_product INIT(PACKAGE_NAME); diff --git a/bin/named/main.c b/bin/named/main.c index 3c12ba0d1d..6751b39432 100644 --- a/bin/named/main.c +++ b/bin/named/main.c @@ -513,7 +513,6 @@ printversion(bool verbose) { isc_buffer_t b; char buf[512]; #if defined(HAVE_GEOIP2) - cfg_parser_t *parser = NULL; cfg_obj_t *config = NULL; const cfg_obj_t *defaults = NULL, *obj = NULL; #endif /* if defined(HAVE_GEOIP2) */ @@ -611,15 +610,13 @@ printversion(bool verbose) { #define RTC(x) RUNTIME_CHECK((x) == ISC_R_SUCCESS) isc_mem_t *geoip_mctx = NULL; isc_mem_create("geoip", &geoip_mctx); - RTC(cfg_parser_create(geoip_mctx, &parser)); - RTC(named_config_parsedefaults(parser, &config)); + RTC(named_config_parsedefaults(&config)); RTC(cfg_map_get(config, "options", &defaults)); RTC(cfg_map_get(defaults, "geoip-directory", &obj)); if (cfg_obj_isstring(obj)) { printf(" geoip-directory: %s\n", cfg_obj_asstring(obj)); } cfg_obj_destroy(&config); - cfg_parser_destroy(&parser); isc_mem_detach(&geoip_mctx); #endif /* HAVE_GEOIP2 */ } diff --git a/bin/named/server.c b/bin/named/server.c index d8654be66d..fd77c10685 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -9529,11 +9529,7 @@ run_server(void *arg) { isc_timer_create(isc_loop_main(), pps_timer_tick, server, &server->pps_timer); - CHECKFATAL(cfg_parser_create(isc_g_mctx, &named_g_parser), - "creating default configuration parser"); - - CHECKFATAL(named_config_parsedefaults(named_g_parser, - &named_g_defaultconfig), + CHECKFATAL(named_config_parsedefaults(&named_g_defaultconfig), "unable to parse defaults config"); CHECKFATAL(cfg_map_get(named_g_defaultconfig, "options", @@ -9595,7 +9591,6 @@ shutdown_server(void *arg) { } cfg_obj_destroy(&named_g_defaultconfig); - cfg_parser_destroy(&named_g_parser); cfg_parser_destroy(&named_g_addparser); (void)named_server_saventa(server); From ea03d743f7f288cae0d8612ab7c31b093e421d1b Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 21 Oct 2025 17:16:39 +0200 Subject: [PATCH 04/12] remove all shared and global parsers Remove all global cfg_parser objects as well as shared parsers between views to dynamically add zones. Instead, parser are transirently created whenever needed. --- bin/named/include/named/globals.h | 1 - bin/named/server.c | 137 +++++++++++++++--------------- 2 files changed, 69 insertions(+), 69 deletions(-) diff --git a/bin/named/include/named/globals.h b/bin/named/include/named/globals.h index c087fe60df..e5ed76d6e6 100644 --- a/bin/named/include/named/globals.h +++ b/bin/named/include/named/globals.h @@ -61,7 +61,6 @@ EXTERN bool named_g_run_done INIT(false); * for really short timers, another for client timers, and one * for zone timers. */ -EXTERN cfg_parser_t *named_g_addparser INIT(NULL); EXTERN const char *named_g_version INIT(PACKAGE_VERSION); EXTERN const char *named_g_product INIT(PACKAGE_NAME); EXTERN const char *named_g_description INIT(PACKAGE_DESCRIPTION); diff --git a/bin/named/server.c b/bin/named/server.c index fd77c10685..b361ab13fb 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -307,8 +307,6 @@ typedef struct matching_view_ctx { */ typedef struct ns_cfgctx { isc_mem_t *mctx; - cfg_parser_t *conf_parser; - cfg_parser_t *add_parser; cfg_obj_t *config; cfg_obj_t *vconfig; cfg_obj_t *nzf_config; @@ -2351,6 +2349,7 @@ catz_addmodzone_cb(void *arg) { cfg_obj_t *zoneobj = NULL; ns_cfgctx_t *cfg = NULL; dns_zone_t *zone = NULL; + cfg_parser_t *parser = NULL; if (isc_loop_shuttingdown(isc_loop_get(isc_tid()))) { goto cleanup; @@ -2465,9 +2464,10 @@ catz_addmodzone_cb(void *arg) { confbuf = NULL; result = dns_catz_generate_zonecfg(cz->origin, cz->entry, &confbuf); if (result == ISC_R_SUCCESS) { - cfg_parser_reset(cfg->add_parser); - result = cfg_parse_buffer(cfg->add_parser, confbuf, "catz", 0, + CHECK(cfg_parser_create(cfg->mctx, &parser)); + result = cfg_parse_buffer(parser, confbuf, "catz", 0, &cfg_type_addzoneconf, 0, &zoneconf); + cfg_parser_destroy(&parser); isc_buffer_free(&confbuf); } /* @@ -2721,6 +2721,7 @@ catz_reconfigure(dns_catz_entry_t *entry, void *arg1, void *arg2) { ns_cfgctx_t *cfg = NULL; dns_zone_t *zone = NULL; isc_result_t result; + cfg_parser_t *parser = NULL; isc_buffer_init(&namebuf, nameb, DNS_NAME_FORMATSIZE); dns_name_totext(dns_catz_entry_getname(entry), DNS_NAME_OMITFINALDOT, @@ -2744,9 +2745,10 @@ catz_reconfigure(dns_catz_entry_t *entry, void *arg1, void *arg2) { result = dns_catz_generate_zonecfg(data->catz, entry, &confbuf); if (result == ISC_R_SUCCESS) { - cfg_parser_reset(cfg->add_parser); - result = cfg_parse_buffer(cfg->add_parser, confbuf, "catz", 0, + CHECK(cfg_parser_create(cfg->mctx, &parser)); + result = cfg_parse_buffer(parser, confbuf, "catz", 0, &cfg_type_addzoneconf, 0, &zoneconf); + cfg_parser_destroy(&parser); isc_buffer_free(&confbuf); } /* @@ -7338,7 +7340,7 @@ cleanup: static isc_result_t setup_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, - cfg_parser_t *config_parser, cfg_aclconfctx_t *aclctx) { + cfg_aclconfctx_t *aclctx) { isc_result_t result = ISC_R_SUCCESS; bool allow = false; ns_cfgctx_t *nzcfg = NULL; @@ -7445,16 +7447,12 @@ setup_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, * a shutdown race later. */ isc_mem_attach(view->mctx, &nzcfg->mctx); - cfg_parser_attach(config_parser, &nzcfg->conf_parser); - cfg_parser_attach(named_g_addparser, &nzcfg->add_parser); cfg_aclconfctx_attach(aclctx, &nzcfg->aclctx); result = dns_view_setnewzones(view, true, nzcfg, newzone_cfgctx_destroy, mapsize); if (result != ISC_R_SUCCESS) { cfg_aclconfctx_detach(&nzcfg->aclctx); - cfg_parser_destroy(&nzcfg->conf_parser); - cfg_parser_destroy(&nzcfg->add_parser); isc_mem_putanddetach(&nzcfg->mctx, nzcfg, sizeof(*nzcfg)); dns_view_setnewzones(view, false, NULL, NULL, 0ULL); return result; @@ -7560,6 +7558,7 @@ data_to_cfg(dns_view_t *view, MDB_val *key, MDB_val *data, isc_buffer_t **text, size_t zone_config_len; cfg_obj_t *zoneconf = NULL; char bufname[DNS_NAME_FORMATSIZE]; + cfg_parser_t *parser = NULL; REQUIRE(view != NULL); REQUIRE(key != NULL); @@ -7597,9 +7596,10 @@ data_to_cfg(dns_view_t *view, MDB_val *key, MDB_val *data, isc_buffer_t **text, snprintf(bufname, sizeof(bufname), "%.*s", (int)zone_name_len, zone_name); - cfg_parser_reset(named_g_addparser); - result = cfg_parse_buffer(named_g_addparser, *text, bufname, 0, + CHECK(cfg_parser_create(view->mctx, &parser)); + result = cfg_parse_buffer(parser, *text, bufname, 0, &cfg_type_addzoneconf, 0, &zoneconf); + cfg_parser_destroy(&parser); if (result != ISC_R_SUCCESS) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, @@ -7857,7 +7857,7 @@ cleanup: ISC_LOG_DEBUG(1), "apply_configuration: %s", __func__); static isc_result_t -create_views(cfg_obj_t *config, cfg_parser_t *parser, cfg_aclconfctx_t *aclctx, +create_views(cfg_obj_t *config, cfg_aclconfctx_t *aclctx, dns_viewlist_t *viewlist) { isc_result_t result = ISC_R_SUCCESS; const cfg_obj_t *views = NULL; @@ -7875,7 +7875,7 @@ create_views(cfg_obj_t *config, cfg_parser_t *parser, cfg_aclconfctx_t *aclctx, } INSIST(view != NULL); - result = setup_newzones(view, config, vconfig, parser, aclctx); + result = setup_newzones(view, config, vconfig, aclctx); dns_view_detach(&view); if (result != ISC_R_SUCCESS) { @@ -7896,7 +7896,7 @@ create_views(cfg_obj_t *config, cfg_parser_t *parser, cfg_aclconfctx_t *aclctx, } INSIST(view != NULL); - result = setup_newzones(view, config, NULL, parser, aclctx); + result = setup_newzones(view, config, NULL, aclctx); dns_view_detach(&view); } @@ -8114,9 +8114,8 @@ configure_kasplist(const cfg_obj_t *config, dns_kasplist_t *kasplist, } static isc_result_t -apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, - cfg_obj_t *bindkeys, named_server_t *server, - bool first_time) { +apply_configuration(cfg_obj_t *config, cfg_obj_t *bindkeys, + named_server_t *server, bool first_time) { const cfg_obj_t *maps[3]; const cfg_obj_t *obj = NULL; const cfg_obj_t *options = NULL; @@ -8192,7 +8191,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, goto cleanup_kasplist; } - result = create_views(config, configparser, aclctx, &viewlist); + result = create_views(config, aclctx, &viewlist); if (result != ISC_R_SUCCESS) { goto cleanup_viewlist; } @@ -9303,15 +9302,8 @@ load_configuration(named_server_t *server, bool first_time) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_DEBUG(1), "load_configuration"); - result = cfg_parser_create(isc_g_mctx, &parser); - if (result != ISC_R_SUCCESS) { - goto out; - } - - result = named_config_parsefile(parser, &config); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } + CHECK(cfg_parser_create(isc_g_mctx, &parser)); + CHECK(named_config_parsefile(parser, &config)); if (named_g_bindkeysfile != NULL) { /* @@ -9343,19 +9335,19 @@ load_configuration(named_server_t *server, bool first_time) { } } - result = apply_configuration(parser, config, bindkeys, server, - first_time); + result = apply_configuration(config, bindkeys, server, first_time); cleanup: + if (parser != NULL) { + cfg_parser_destroy(&parser); + } if (bindkeys != NULL) { cfg_obj_destroy(&bindkeys); } if (config != NULL) { cfg_obj_destroy(&config); } - cfg_parser_destroy(&parser); -out: return result; } @@ -9536,9 +9528,6 @@ run_server(void *arg) { &named_g_defaultoptions), "missing 'options' in default config"); - CHECKFATAL(cfg_parser_create(isc_g_mctx, &named_g_addparser), - "creating additional configuration parser"); - CHECKFATAL(load_configuration(server, true), "loading configuration"); CHECKFATAL(load_zones(server, false), "loading zones"); @@ -9591,7 +9580,6 @@ shutdown_server(void *arg) { } cfg_obj_destroy(&named_g_defaultconfig); - cfg_parser_destroy(&named_g_addparser); (void)named_server_saventa(server); @@ -12892,6 +12880,7 @@ load_nzf(dns_view_t *view, ns_cfgctx_t *nzcfg) { MDB_dbi dbi; MDB_val key, data; ns_dzarg_t dzarg; + cfg_parser_t *parser = NULL; UNUSED(nzcfg); @@ -12915,9 +12904,10 @@ load_nzf(dns_view_t *view, ns_cfgctx_t *nzcfg) { * config type, giving us a guarantee that valid configuration * will be written to DB. */ - cfg_parser_reset(named_g_addparser); - result = cfg_parse_file(named_g_addparser, view->new_zone_file, + CHECK(cfg_parser_create(nzcfg->mctx, &parser)); + result = cfg_parse_file(parser, view->new_zone_file, &cfg_type_addzoneconf, &nzf_config); + cfg_parser_destroy(&parser); if (result != ISC_R_SUCCESS) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, "Error parsing NZF file '%s': %s", @@ -13047,6 +13037,7 @@ newzone_parse(named_server_t *server, char *command, dns_view_t **viewp, dns_rdataclass_t rdclass; dns_view_t *view = NULL; const char *bn = NULL; + cfg_parser_t *parser = NULL; REQUIRE(viewp != NULL && *viewp == NULL); REQUIRE(zoneobjp != NULL && *zoneobjp == NULL); @@ -13071,9 +13062,11 @@ newzone_parse(named_server_t *server, char *command, dns_view_t **viewp, */ isc_buffer_forward(&argbuf, 3); - cfg_parser_reset(named_g_addparser); - CHECK(cfg_parse_buffer(named_g_addparser, &argbuf, bn, 0, - &cfg_type_addzoneconf, 0, &zoneconf)); + CHECK(cfg_parser_create(server->mctx, &parser)); + CHECK(cfg_parse_buffer(parser, &argbuf, bn, 0, &cfg_type_addzoneconf, 0, + &zoneconf)); + cfg_parser_destroy(&parser); + CHECK(cfg_map_get(zoneconf, "zone", &zlist)); if (!cfg_obj_islist(zlist)) { CHECK(ISC_R_FAILURE); @@ -13147,6 +13140,10 @@ newzone_parse(named_server_t *server, char *command, dns_view_t **viewp, return ISC_R_SUCCESS; cleanup: + if (parser != NULL) { + cfg_parser_destroy(&parser); + } + if (zoneconf != NULL) { cfg_obj_destroy(&zoneconf); } @@ -13408,6 +13405,7 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, dns_zone_t *zone = NULL; const cfg_obj_t *voptions = NULL; bool added; + cfg_parser_t *parser = NULL; #ifndef HAVE_LMDB FILE *fp = NULL; cfg_obj_t *z; @@ -13527,16 +13525,18 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, #endif /* HAVE_LMDB */ if (!added) { + TCHECK(cfg_parser_create(cfg->mctx, &parser)); if (cfg->vconfig == NULL) { - result = delete_zoneconf( - view, cfg->conf_parser, cfg->config, - dns_zone_getorigin(zone), NULL); + result = delete_zoneconf(view, parser, cfg->config, + dns_zone_getorigin(zone), + NULL); } else { voptions = cfg_tuple_get(cfg->vconfig, "options"); - result = delete_zoneconf( - view, cfg->conf_parser, voptions, - dns_zone_getorigin(zone), NULL); + result = delete_zoneconf(view, parser, voptions, + dns_zone_getorigin(zone), + NULL); } + cfg_parser_destroy(&parser); if (result != ISC_R_SUCCESS) { TCHECK(putstr(text, "former zone configuration " @@ -13611,7 +13611,6 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, } cleanup: - #ifndef HAVE_LMDB if (fp != NULL) { (void)isc_stdio_close(fp); @@ -13760,6 +13759,7 @@ rmzone(void *arg) { dns_db_t *dbp = NULL; bool added; isc_result_t result; + cfg_parser_t *parser = NULL; #ifdef HAVE_LMDB MDB_txn *txn = NULL; MDB_dbi dbi; @@ -13823,17 +13823,24 @@ rmzone(void *arg) { } if (!added && cfg != NULL) { + result = cfg_parser_create(cfg->mctx, &parser); + INSIST(result == ISC_R_SUCCESS); /* colin: this is just a + shortcut until the + initialization function of + the parser can't fails */ + if (cfg->vconfig != NULL) { const cfg_obj_t *voptions = cfg_tuple_get(cfg->vconfig, "options"); - result = delete_zoneconf( - view, cfg->conf_parser, voptions, - dns_zone_getorigin(zone), NULL); + result = delete_zoneconf(view, parser, voptions, + dns_zone_getorigin(zone), + NULL); } else { - result = delete_zoneconf( - view, cfg->conf_parser, cfg->config, - dns_zone_getorigin(zone), NULL); + result = delete_zoneconf(view, parser, cfg->config, + dns_zone_getorigin(zone), + NULL); } + cfg_parser_destroy(&parser); if (result != ISC_R_SUCCESS) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, @@ -14225,20 +14232,14 @@ newzone_cfgctx_destroy(void **cfgp) { cfg = *cfgp; - if (cfg->conf_parser != NULL) { - if (cfg->config != NULL) { - cfg_obj_destroy(&cfg->config); - } - if (cfg->vconfig != NULL) { - cfg_obj_destroy(&cfg->vconfig); - } - cfg_parser_destroy(&cfg->conf_parser); + if (cfg->config != NULL) { + cfg_obj_destroy(&cfg->config); } - if (cfg->add_parser != NULL) { - if (cfg->nzf_config != NULL) { - cfg_obj_destroy(&cfg->nzf_config); - } - cfg_parser_destroy(&cfg->add_parser); + if (cfg->vconfig != NULL) { + cfg_obj_destroy(&cfg->vconfig); + } + if (cfg->nzf_config != NULL) { + cfg_obj_destroy(&cfg->nzf_config); } if (cfg->aclctx != NULL) { From 6de1d0dbc4101f24ead3578eacb0fecf22b60152 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 21 Oct 2025 16:41:11 -0700 Subject: [PATCH 05/12] update cfg_obj_attach/destroy now that cfg_obj_destroy() has been simplified, we can use the ISC_REFCOUNT macros to declare cfg_obj_attach() and _detach(). --- bin/check/named-checkconf.c | 2 +- bin/delv/delv.c | 2 +- bin/dig/dighost.c | 2 +- bin/dnssec/dnssec-keygen.c | 2 +- bin/dnssec/dnssec-ksr.c | 2 +- bin/named/config.c | 2 +- bin/named/controlconf.c | 2 +- bin/named/main.c | 2 +- bin/named/server.c | 34 ++++++------ bin/nsupdate/nsupdate.c | 2 +- bin/plugins/filter-a.c | 4 +- bin/plugins/filter-aaaa.c | 4 +- bin/plugins/synthrecord.c | 2 +- bin/rndc/rndc.c | 2 +- .../system/hooks/driver/test-syncplugin.c | 2 +- doc/misc/cfg_test.c | 2 +- lib/isccfg/include/isccfg/cfg.h | 19 +------ lib/isccfg/namedconf.c | 8 +-- lib/isccfg/parser.c | 54 ++++++++----------- tests/isccfg/duration_test.c | 2 +- tests/isccfg/grammar_test.c | 2 +- tests/isccfg/parser_test.c | 6 +-- 22 files changed, 66 insertions(+), 93 deletions(-) diff --git a/bin/check/named-checkconf.c b/bin/check/named-checkconf.c index 4f7bfcee56..0c217efd92 100644 --- a/bin/check/named-checkconf.c +++ b/bin/check/named-checkconf.c @@ -745,7 +745,7 @@ main(int argc, char **argv) { cleanup: if (config != NULL) { - cfg_obj_destroy(&config); + cfg_obj_detach(&config); } if (parser != NULL) { diff --git a/bin/delv/delv.c b/bin/delv/delv.c index 997c9fb73b..dd53c825fa 100644 --- a/bin/delv/delv.c +++ b/bin/delv/delv.c @@ -862,7 +862,7 @@ setup_dnsseckeys(dns_client_t *client, dns_view_t *toview) { cleanup: if (bindkeys != NULL) { - cfg_obj_destroy(&bindkeys); + cfg_obj_detach(&bindkeys); } if (parser != NULL) { cfg_parser_destroy(&parser); diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index a79bf61f9d..51949c2cc9 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -1119,7 +1119,7 @@ read_confkey(void) { cleanup: if (pctx != NULL) { if (file != NULL) { - cfg_obj_destroy(&file); + cfg_obj_detach(&file); } cfg_parser_destroy(&pctx); } diff --git a/bin/dnssec/dnssec-keygen.c b/bin/dnssec/dnssec-keygen.c index 04b8a8056f..ab53148b44 100644 --- a/bin/dnssec/dnssec-keygen.c +++ b/bin/dnssec/dnssec-keygen.c @@ -1199,7 +1199,7 @@ main(int argc, char **argv) { } dns_kasp_detach(&kasp); - cfg_obj_destroy(&config); + cfg_obj_detach(&config); cfg_parser_destroy(&parser); } } else { diff --git a/bin/dnssec/dnssec-ksr.c b/bin/dnssec/dnssec-ksr.c index 073bbda1a9..0a43fd889a 100644 --- a/bin/dnssec/dnssec-ksr.c +++ b/bin/dnssec/dnssec-ksr.c @@ -187,7 +187,7 @@ getkasp(ksr_ctx_t *ksr, dns_kasp_t **kasp) { if (ISC_LIST_EMPTY(dns_kasp_keys(*kasp))) { fatal("dnssec-policy '%s' has no keys configured", ksr->policy); } - cfg_obj_destroy(&config); + cfg_obj_detach(&config); cfg_parser_destroy(&parser); } diff --git a/bin/named/config.c b/bin/named/config.c index d08e94c959..d7022be01b 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -467,7 +467,7 @@ named_config_parsefile(cfg_parser_t *parser, cfg_obj_t **conf) { cleanup: if (*conf) { - cfg_obj_destroy(conf); + cfg_obj_detach(conf); } out: diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c index 26b0fb9197..0ae02c8891 100644 --- a/bin/named/controlconf.c +++ b/bin/named/controlconf.c @@ -861,7 +861,7 @@ cleanup: free_controlkey(keyid, mctx); } if (config != NULL) { - cfg_obj_destroy(&config); + cfg_obj_detach(&config); } if (pctx != NULL) { cfg_parser_destroy(&pctx); diff --git a/bin/named/main.c b/bin/named/main.c index 6751b39432..5f171385c8 100644 --- a/bin/named/main.c +++ b/bin/named/main.c @@ -616,7 +616,7 @@ printversion(bool verbose) { if (cfg_obj_isstring(obj)) { printf(" geoip-directory: %s\n", cfg_obj_asstring(obj)); } - cfg_obj_destroy(&config); + cfg_obj_detach(&config); isc_mem_detach(&geoip_mctx); #endif /* HAVE_GEOIP2 */ } diff --git a/bin/named/server.c b/bin/named/server.c index b361ab13fb..936d397975 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -2544,7 +2544,7 @@ cleanup: dns_zone_detach(&zone); } if (zoneconf != NULL) { - cfg_obj_destroy(&zoneconf); + cfg_obj_detach(&zoneconf); } if (dnsforwarders != NULL) { dns_forwarders_detach(&dnsforwarders); @@ -2785,7 +2785,7 @@ catz_reconfigure(dns_catz_entry_t *entry, void *arg1, void *arg2) { cleanup: if (zoneconf != NULL) { - cfg_obj_destroy(&zoneconf); + cfg_obj_detach(&zoneconf); } dns_zone_detach(&zone); @@ -7615,7 +7615,7 @@ data_to_cfg(dns_view_t *view, MDB_val *key, MDB_val *data, isc_buffer_t **text, cleanup: if (zoneconf != NULL) { - cfg_obj_destroy(&zoneconf); + cfg_obj_detach(&zoneconf); } return result; @@ -7696,14 +7696,14 @@ for_all_newzone_cfgs(newzone_cfg_cb_t callback, cfg_obj_t *config, /* * Destroy the configuration object created in this iteration. */ - cfg_obj_destroy(&zconfigobj); + cfg_obj_detach(&zconfigobj); } if (text != NULL) { isc_buffer_free(&text); } if (zconfigobj != NULL) { - cfg_obj_destroy(&zconfigobj); + cfg_obj_detach(&zconfigobj); } mdb_cursor_close(cursor); @@ -7841,7 +7841,7 @@ cleanup: UNLOCK(&view->new_zone_lock); if (zoneconf != NULL) { - cfg_obj_destroy(&zoneconf); + cfg_obj_detach(&zoneconf); } if (text != NULL) { isc_buffer_free(&text); @@ -9342,10 +9342,10 @@ cleanup: cfg_parser_destroy(&parser); } if (bindkeys != NULL) { - cfg_obj_destroy(&bindkeys); + cfg_obj_detach(&bindkeys); } if (config != NULL) { - cfg_obj_destroy(&config); + cfg_obj_detach(&config); } return result; @@ -9579,7 +9579,7 @@ shutdown_server(void *arg) { cfg_aclconfctx_detach(&server->aclctx); } - cfg_obj_destroy(&named_g_defaultconfig); + cfg_obj_detach(&named_g_defaultconfig); (void)named_server_saventa(server); @@ -13014,7 +13014,7 @@ cleanup: } if (nzf_config != NULL) { - cfg_obj_destroy(&nzf_config); + cfg_obj_detach(&nzf_config); } return result; @@ -13145,7 +13145,7 @@ cleanup: } if (zoneconf != NULL) { - cfg_obj_destroy(&zoneconf); + cfg_obj_detach(&zoneconf); } if (view != NULL) { dns_view_detach(&view); @@ -13189,7 +13189,7 @@ delete_zoneconf(dns_view_t *view, cfg_parser_t *pctx, const cfg_obj_t *config, e = UNCONST(elt); ISC_LIST_UNLINK(*list, e, link); - cfg_obj_destroy(&e->obj); + cfg_obj_detach(&e->obj); isc_mem_put(pctx->mctx, e, sizeof(*e)); result = ISC_R_SUCCESS; break; @@ -13715,7 +13715,7 @@ cleanup: (void)putnull(text); } if (zoneconf != NULL) { - cfg_obj_destroy(&zoneconf); + cfg_obj_detach(&zoneconf); } if (view != NULL) { dns_view_detach(&view); @@ -14214,7 +14214,7 @@ named_server_showzone(named_server_t *server, isc_lex_t *lex, cleanup: #ifdef HAVE_LMDB if (nzconfig != NULL) { - cfg_obj_destroy(&nzconfig); + cfg_obj_detach(&nzconfig); } #endif /* HAVE_LMDB */ if (isc_buffer_usedlength(*text) > 0) { @@ -14233,13 +14233,13 @@ newzone_cfgctx_destroy(void **cfgp) { cfg = *cfgp; if (cfg->config != NULL) { - cfg_obj_destroy(&cfg->config); + cfg_obj_detach(&cfg->config); } if (cfg->vconfig != NULL) { - cfg_obj_destroy(&cfg->vconfig); + cfg_obj_detach(&cfg->vconfig); } if (cfg->nzf_config != NULL) { - cfg_obj_destroy(&cfg->nzf_config); + cfg_obj_detach(&cfg->nzf_config); } if (cfg->aclctx != NULL) { diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index c9d9ce0adf..585dc80b91 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -604,7 +604,7 @@ read_sessionkey(isc_mem_t *mctx) { cleanup: if (pctx != NULL) { if (sessionkey != NULL) { - cfg_obj_destroy(&sessionkey); + cfg_obj_detach(&sessionkey); } cfg_parser_destroy(&pctx); } diff --git a/bin/plugins/filter-a.c b/bin/plugins/filter-a.c index 8b99c6c4bc..5d1cf06e52 100644 --- a/bin/plugins/filter-a.c +++ b/bin/plugins/filter-a.c @@ -300,7 +300,7 @@ parse_parameters(filter_instance_t *inst, const char *parameters, cleanup: if (param_obj != NULL) { - cfg_obj_destroy(¶m_obj); + cfg_obj_detach(¶m_obj); } if (parser != NULL) { cfg_parser_destroy(&parser); @@ -383,7 +383,7 @@ plugin_check(const char *parameters, const void *cfg, const char *cfg_file, cleanup: if (param_obj != NULL) { - cfg_obj_destroy(¶m_obj); + cfg_obj_detach(¶m_obj); } if (parser != NULL) { cfg_parser_destroy(&parser); diff --git a/bin/plugins/filter-aaaa.c b/bin/plugins/filter-aaaa.c index b9a2b52d54..ad2ef52f3a 100644 --- a/bin/plugins/filter-aaaa.c +++ b/bin/plugins/filter-aaaa.c @@ -303,7 +303,7 @@ parse_parameters(filter_instance_t *inst, const char *parameters, cleanup: if (param_obj != NULL) { - cfg_obj_destroy(¶m_obj); + cfg_obj_detach(¶m_obj); } if (parser != NULL) { cfg_parser_destroy(&parser); @@ -387,7 +387,7 @@ plugin_check(const char *parameters, const void *cfg, const char *cfg_file, cleanup: if (param_obj != NULL) { - cfg_obj_destroy(¶m_obj); + cfg_obj_detach(¶m_obj); } if (parser != NULL) { cfg_parser_destroy(&parser); diff --git a/bin/plugins/synthrecord.c b/bin/plugins/synthrecord.c index e963cdad5b..5c5f34bf89 100644 --- a/bin/plugins/synthrecord.c +++ b/bin/plugins/synthrecord.c @@ -593,7 +593,7 @@ synthrecord_parseconfig(synthrecord_t *inst, const char *parameters, cleanup: if (synthrecordcfg != NULL) { - cfg_obj_destroy(&synthrecordcfg); + cfg_obj_detach(&synthrecordcfg); } if (parser != NULL) { diff --git a/bin/rndc/rndc.c b/bin/rndc/rndc.c index 6aa1e4e67b..3204e30a63 100644 --- a/bin/rndc/rndc.c +++ b/bin/rndc/rndc.c @@ -999,7 +999,7 @@ main(int argc, char **argv) { isccc_ccmsg_invalidate(&rndc_ccmsg); - cfg_obj_destroy(&config); + cfg_obj_detach(&config); cfg_parser_destroy(&pctx); isc_mem_put(isc_g_mctx, args, argslen); diff --git a/bin/tests/system/hooks/driver/test-syncplugin.c b/bin/tests/system/hooks/driver/test-syncplugin.c index 86bea21cf4..09856fd2cc 100644 --- a/bin/tests/system/hooks/driver/test-syncplugin.c +++ b/bin/tests/system/hooks/driver/test-syncplugin.c @@ -217,7 +217,7 @@ cleanup: } if (syncplugincfg != NULL) { - cfg_obj_destroy(&syncplugincfg); + cfg_obj_detach(&syncplugincfg); } if (parser != NULL) { diff --git a/doc/misc/cfg_test.c b/doc/misc/cfg_test.c index db596e3df8..60b28bc159 100644 --- a/doc/misc/cfg_test.c +++ b/doc/misc/cfg_test.c @@ -146,7 +146,7 @@ main(int argc, char **argv) { cfg_print(cfg, output, NULL); - cfg_obj_destroy(&cfg); + cfg_obj_detach(&cfg); cfg_parser_destroy(&pctx); } diff --git a/lib/isccfg/include/isccfg/cfg.h b/lib/isccfg/include/isccfg/cfg.h index 77576fcf2b..8e861655a5 100644 --- a/lib/isccfg/include/isccfg/cfg.h +++ b/lib/isccfg/include/isccfg/cfg.h @@ -550,23 +550,6 @@ cfg_obj_istype(const cfg_obj_t *obj, const cfg_type_t *type); * Return true iff 'obj' is of type 'type'. */ -void -cfg_obj_attach(cfg_obj_t *src, cfg_obj_t **dest); -/*%< - * Reference a configuration object. - */ - -void -cfg_obj_destroy(cfg_obj_t **obj); -/*%< - * Delete a reference to a configuration object; destroy the object if - * there are no more references. - * - * Require: - * \li '*obj' is a valid cfg_obj_t. - * \li 'mctx' is a valid isc_mem_t. - */ - void cfg_obj_log(const cfg_obj_t *obj, int level, const char *fmt, ...) ISC_FORMAT_PRINTF(3, 4); @@ -633,3 +616,5 @@ cfg_pluginlist_foreach(const cfg_obj_t *config, const cfg_obj_t *list, * 'list' * \li first 'callback' return value which was not #ISC_R_SUCCESS otherwise */ + +ISC_REFCOUNT_DECL(cfg_obj); diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c index 46e0c54d6d..ce30d4b948 100644 --- a/lib/isccfg/namedconf.c +++ b/lib/isccfg/namedconf.c @@ -42,10 +42,10 @@ } while (0) /*% Clean up a configuration object if non-NULL. */ -#define CLEANUP_OBJ(obj) \ - do { \ - if ((obj) != NULL) \ - cfg_obj_destroy(&(obj)); \ +#define CLEANUP_OBJ(obj) \ + do { \ + if ((obj) != NULL) \ + cfg_obj_detach(&(obj)); \ } while (0) /*% diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index 9042493bb0..822ccc83a2 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -88,10 +88,10 @@ } while (0) /* Clean up a configuration object if non-NULL. */ -#define CLEANUP_OBJ(obj) \ - do { \ - if ((obj) != NULL) \ - cfg_obj_destroy(&(obj)); \ +#define CLEANUP_OBJ(obj) \ + do { \ + if ((obj) != NULL) \ + cfg_obj_detach(&(obj)); \ } while (0) /* cfg_obj_t magic number */ @@ -1973,7 +1973,7 @@ create_listelt(cfg_parser_t *pctx, cfg_listelt_t **eltp) { static void free_listelt(isc_mem_t *mctx, cfg_listelt_t *elt) { if (elt->obj != NULL) { - cfg_obj_destroy(&elt->obj); + cfg_obj_detach(&elt->obj); } isc_mem_put(mctx, elt, sizeof(*elt)); } @@ -2305,7 +2305,7 @@ cfg_parse_mapbody(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { CHECK(parser_openfile(pctx, g.gl_pathv[i])); } - cfg_obj_destroy(&includename); + cfg_obj_detach(&includename); globfree(&g); goto redo; @@ -2333,7 +2333,7 @@ cfg_parse_mapbody(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { */ CHECK(cfg_parse_obj(pctx, &cfg_type_unsupported, &eltobj)); - cfg_obj_destroy(&eltobj); + cfg_obj_detach(&eltobj); CHECK(parse_semicolon(pctx)); continue; } @@ -3769,7 +3769,7 @@ map_symtabitem_destroy(char *key, unsigned int type, isc_symvalue_t symval, UNUSED(type); UNUSED(userarg); - cfg_obj_destroy(&obj); + cfg_obj_detach(&obj); } static isc_result_t @@ -3812,36 +3812,24 @@ cfg_obj_istype(const cfg_obj_t *obj, const cfg_type_t *type) { /* * Destroy 'obj', a configuration object created in 'pctx'. */ -void -cfg_obj_destroy(cfg_obj_t **objp) { - REQUIRE(objp != NULL && *objp != NULL); - REQUIRE((*objp)->magic == CFGOBJ_MAGIC); +static void +cfg__obj_destroy(cfg_obj_t *obj) { + REQUIRE(obj != NULL); + REQUIRE(obj->magic == CFGOBJ_MAGIC); - cfg_obj_t *obj = *objp; - *objp = NULL; + obj->magic = 0; - if (isc_refcount_decrement(&obj->references) == 1) { - obj->magic = 0; - - if (obj->file != NULL) { - cfg_obj_destroy(&obj->file); - } - - obj->type->rep->free(obj); - - isc_refcount_destroy(&obj->references); - isc_mem_putanddetach(&obj->mctx, obj, sizeof(cfg_obj_t)); + if (obj->file != NULL) { + cfg_obj_detach(&obj->file); } + + obj->type->rep->free(obj); + + isc_refcount_destroy(&obj->references); + isc_mem_putanddetach(&obj->mctx, obj, sizeof(cfg_obj_t)); } -void -cfg_obj_attach(cfg_obj_t *src, cfg_obj_t **dest) { - REQUIRE(src != NULL); - REQUIRE(dest != NULL && *dest == NULL); - - isc_refcount_increment(&src->references); - *dest = src; -} +ISC_REFCOUNT_IMPL(cfg_obj, cfg__obj_destroy); static void free_noop(cfg_obj_t *obj) { diff --git a/tests/isccfg/duration_test.c b/tests/isccfg/duration_test.c index a9524a0c4a..4454933a1c 100644 --- a/tests/isccfg/duration_test.c +++ b/tests/isccfg/duration_test.c @@ -192,7 +192,7 @@ ISC_RUN_TEST_IMPL(duration) { assert_int_equal(cmp, 0); } - cfg_obj_destroy(&c1); + cfg_obj_detach(&c1); cfg_parser_destroy(&p1); } } diff --git a/tests/isccfg/grammar_test.c b/tests/isccfg/grammar_test.c index d2424b2afe..ac06a7e49a 100644 --- a/tests/isccfg/grammar_test.c +++ b/tests/isccfg/grammar_test.c @@ -144,7 +144,7 @@ test__query_source_print(const char *config, const char *expected) { output_conf->type->print(&pctx, output_conf); assert_text(expected); - cfg_obj_destroy(parser, &output_conf); + cfg_obj_detach(parser, &output_conf); cfg_parser_reset(parser); cfg_parser_destroy(&parser); } diff --git a/tests/isccfg/parser_test.c b/tests/isccfg/parser_test.c index 7efc320dae..8ace4ea4f4 100644 --- a/tests/isccfg/parser_test.c +++ b/tests/isccfg/parser_test.c @@ -102,7 +102,7 @@ ISC_RUN_TEST_IMPL(addzoneconf) { strlcat(buf, ";", sizeof(buf)); assert_string_equal(tests[i], buf); - cfg_obj_destroy(&conf); + cfg_obj_detach(&conf); cfg_parser_reset(p); } @@ -141,8 +141,8 @@ ISC_RUN_TEST_IMPL(parse_buffer) { assert_int_equal(result, ISC_R_SUCCESS); assert_int_equal(p2->line, 104); - cfg_obj_destroy(&c1); - cfg_obj_destroy(&c2); + cfg_obj_detach(&c1); + cfg_obj_detach(&c2); cfg_parser_destroy(&p1); cfg_parser_destroy(&p2); From 0191ba5540e5bdfeddc275755bf72c26722d38ec Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 21 Oct 2025 22:56:27 -0700 Subject: [PATCH 06/12] store the zone configuration object in the zone when configuring a zone, we can now save the zone's configuration object in the zone itself by calling dns_zone_setcfg(). this can then be used by "rndc showzone" to print the zone's configuration, which is simpler than searching for it using the new-zones configuration, and allows it to work even if "allow-new-zones" is disabled. --- bin/named/server.c | 190 +------------------------------------ bin/named/zoneconf.c | 11 +++ lib/dns/include/dns/zone.h | 19 ++++ lib/dns/zone.c | 26 +++++ 4 files changed, 59 insertions(+), 187 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 936d397975..5fdfedd082 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -7546,7 +7546,7 @@ cleanup: return result; } -#else /* HAVE_LMDB */ +#else /* HAVE_LMDB */ static isc_result_t data_to_cfg(dns_view_t *view, MDB_val *key, MDB_val *data, isc_buffer_t **text, @@ -7786,70 +7786,6 @@ configure_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, return result; } - -static isc_result_t -get_newzone_config(dns_view_t *view, const char *zonename, - cfg_obj_t **zoneconfig) { - isc_result_t result; - int status; - cfg_obj_t *zoneconf = NULL; - isc_buffer_t *text = NULL; - MDB_txn *txn = NULL; - MDB_dbi dbi; - MDB_val key, data; - char zname[DNS_NAME_FORMATSIZE]; - dns_fixedname_t fname; - dns_name_t *name; - isc_buffer_t b; - - INSIST(zoneconfig != NULL && *zoneconfig == NULL); - - LOCK(&view->new_zone_lock); - - CHECK(nzd_open(view, MDB_RDONLY, &txn, &dbi)); - - isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, - ISC_LOG_INFO, - "loading NZD config from '%s' " - "for zone '%s'", - view->new_zone_db, zonename); - - /* Normalize zone name */ - isc_buffer_constinit(&b, zonename, strlen(zonename)); - isc_buffer_add(&b, strlen(zonename)); - name = dns_fixedname_initname(&fname); - CHECK(dns_name_fromtext(name, &b, dns_rootname, DNS_NAME_DOWNCASE)); - dns_name_format(name, zname, sizeof(zname)); - - key.mv_data = zname; - key.mv_size = strlen(zname); - - status = mdb_get(txn, dbi, &key, &data); - if (status != MDB_SUCCESS) { - CHECK(ISC_R_FAILURE); - } - - CHECK(data_to_cfg(view, &key, &data, &text, &zoneconf)); - - *zoneconfig = zoneconf; - zoneconf = NULL; - result = ISC_R_SUCCESS; - -cleanup: - (void)nzd_close(&txn, false); - - UNLOCK(&view->new_zone_lock); - - if (zoneconf != NULL) { - cfg_obj_detach(&zoneconf); - } - if (text != NULL) { - isc_buffer_free(&text); - } - - return result; -} - #endif /* HAVE_LMDB */ #define APPLY_CONFIGURATION_SUBROUTINE_LOG \ @@ -14045,72 +13981,6 @@ cleanup: return result; } -static const cfg_obj_t * -find_name_in_list_from_map(const cfg_obj_t *config, - const char *map_key_for_list, const char *name, - bool redirect) { - const cfg_obj_t *list = NULL; - const cfg_obj_t *obj = NULL; - dns_fixedname_t fixed1, fixed2; - dns_name_t *name1 = NULL, *name2 = NULL; - isc_result_t result; - - if (strcmp(map_key_for_list, "zone") == 0) { - name1 = dns_fixedname_initname(&fixed1); - name2 = dns_fixedname_initname(&fixed2); - result = dns_name_fromstring(name1, name, dns_rootname, 0, - NULL); - RUNTIME_CHECK(result == ISC_R_SUCCESS); - } - - cfg_map_get(config, map_key_for_list, &list); - CFG_LIST_FOREACH(list, element) { - const char *vname = NULL; - - obj = cfg_listelt_value(element); - INSIST(obj != NULL); - vname = cfg_obj_asstring(cfg_tuple_get(obj, "name")); - if (vname == NULL) { - obj = NULL; - continue; - } - - if (name1 != NULL) { - result = dns_name_fromstring(name2, vname, dns_rootname, - 0, NULL); - if (result == ISC_R_SUCCESS && - dns_name_equal(name1, name2)) - { - const cfg_obj_t *zoptions = - cfg_tuple_get(obj, "options"); - const cfg_obj_t *typeobj = NULL; - - if (zoptions != NULL) { - const cfg_obj_t *toptions = - named_zone_templateopts( - config, zoptions); - named_config_findopt(zoptions, toptions, - "type", &typeobj); - } - if (redirect && typeobj != NULL && - strcasecmp(cfg_obj_asstring(typeobj), - "redirect") == 0) - { - break; - } else if (!redirect) { - break; - } - } - } else if (strcasecmp(vname, name) == 0) { - break; - } - - obj = NULL; - } - - return obj; -} - static void emitzone(void *arg, const char *buf, int len) { ns_dzarg_t *dzarg = arg; @@ -14130,16 +14000,9 @@ isc_result_t named_server_showzone(named_server_t *server, isc_lex_t *lex, isc_buffer_t **text) { isc_result_t result; - const cfg_obj_t *vconfig = NULL, *zconfig = NULL; + const cfg_obj_t *zconfig = NULL; char zonename[DNS_NAME_FORMATSIZE]; - const cfg_obj_t *map; - dns_view_t *view = NULL; dns_zone_t *zone = NULL; - ns_cfgctx_t *cfg = NULL; -#ifdef HAVE_LMDB - cfg_obj_t *nzconfig = NULL; -#endif /* HAVE_LMDB */ - bool added, redirect; ns_dzarg_t dzarg; REQUIRE(text != NULL); @@ -14151,51 +14014,9 @@ named_server_showzone(named_server_t *server, isc_lex_t *lex, goto cleanup; } - redirect = dns_zone_gettype(zone) == dns_zone_redirect; - added = dns_zone_getadded(zone); - view = dns_zone_getview(zone); + zconfig = dns_zone_getcfg(zone); dns_zone_detach(&zone); - cfg = (ns_cfgctx_t *)view->new_zone_config; - if (cfg == NULL) { - result = ISC_R_FAILURE; - goto cleanup; - } - - if (!added) { - /* Find the view statement */ - vconfig = find_name_in_list_from_map(cfg->config, "view", - view->name, false); - - /* Find the zone statement */ - if (vconfig != NULL) { - map = cfg_tuple_get(vconfig, "options"); - } else { - map = cfg->config; - } - - zconfig = find_name_in_list_from_map(map, "zone", zonename, - redirect); - } - -#ifndef HAVE_LMDB - if (zconfig == NULL && cfg->nzf_config != NULL) { - zconfig = find_name_in_list_from_map(cfg->nzf_config, "zone", - zonename, redirect); - } -#else /* HAVE_LMDB */ - if (zconfig == NULL) { - const cfg_obj_t *zlist = NULL; - CHECK(get_newzone_config(view, zonename, &nzconfig)); - CHECK(cfg_map_get(nzconfig, "zone", &zlist)); - if (!cfg_obj_islist(zlist)) { - CHECK(ISC_R_FAILURE); - } - - zconfig = cfg_listelt_value(cfg_list_first(zlist)); - } -#endif /* HAVE_LMDB */ - if (zconfig == NULL) { CHECK(ISC_R_NOTFOUND); } @@ -14212,11 +14033,6 @@ named_server_showzone(named_server_t *server, isc_lex_t *lex, result = ISC_R_SUCCESS; cleanup: -#ifdef HAVE_LMDB - if (nzconfig != NULL) { - cfg_obj_detach(&nzconfig); - } -#endif /* HAVE_LMDB */ if (isc_buffer_usedlength(*text) > 0) { (void)putnull(text); } diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c index ae3d95bda1..ae82d76feb 100644 --- a/bin/named/zoneconf.c +++ b/bin/named/zoneconf.c @@ -869,6 +869,12 @@ process_notifytype(dns_notifytype_t ntype, dns_zonetype_t ztype, return dns_notifytype_explicit; } +static void +detach_cfg(void *arg) { + cfg_obj_t *cfg = (cfg_obj_t *)arg; + cfg_obj_detach(&cfg); +} + isc_result_t named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig, const cfg_obj_t *zconfig, cfg_aclconfctx_t *aclctx, @@ -1908,6 +1914,11 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig, break; } + /* Save the configuration for later use */ + cfg_obj_t *cfg = UNCONST(zconfig); + cfg_obj_ref(cfg); + dns_zone_setcfg(zone, (void *)cfg, detach_cfg); + result = ISC_R_SUCCESS; cleanup: diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index 24cd59fd16..7a7a8325cf 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -2777,6 +2777,25 @@ dns_zone_unloadplugins(dns_zone_t *zone); * \li 'zone' to be a valid zone. */ +void +dns_zone_setcfg(dns_zone_t *zone, void *cfg, void (*cfg_detach)(void *)); +/*%< + * Set a pointer to the configuration object for 'zone', which can be + * used later to dump the configuration status. + * + * Requires: + * \li 'zone' to be a valid zone. + */ +void * +dns_zone_getcfg(dns_zone_t *zone); +/*%< + * Return a pointer to the configuration object for 'zone', that was + * previously set using _setcfg(). + * + * Requires: + * \li 'zone' to be a valid zone. + */ + #if DNS_ZONE_TRACE #define dns_zone_ref(ptr) dns_zone__ref(ptr, __func__, __FILE__, __LINE__) #define dns_zone_unref(ptr) dns_zone__unref(ptr, __func__, __FILE__, __LINE__) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index b14b7fffb4..d4275f24d3 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -538,6 +538,10 @@ struct dns_zone { void (*plugins_free)(isc_mem_t *, void **); void *hooktable; void (*hooktable_free)(isc_mem_t *, void **); + + /* Configuration object */ + void *cfg; + void (*cfg_detach)(void *); }; #define zonediff_init(z, d) \ @@ -15343,6 +15347,9 @@ zone_shutdown(void *arg) { dns_zonemgr_releasezone(zone->zmgr, zone); } + /* Detach the zone configuration pointer */ + dns_zone_setcfg(zone, NULL, NULL); + LOCK_ZONE(zone); INSIST(zone != zone->raw); @@ -24908,3 +24915,22 @@ dns_zone_unloadplugins(dns_zone_t *zone) { zone->plugins_free = NULL; } } + +void +dns_zone_setcfg(dns_zone_t *zone, void *cfg, void (*cfg_detach)(void *)) { + REQUIRE(DNS_ZONE_VALID(zone)); + + if (zone->cfg != NULL && zone->cfg_detach != NULL) { + zone->cfg_detach(zone->cfg); + zone->cfg = NULL; + } + zone->cfg = cfg; + zone->cfg_detach = cfg_detach; +} + +void * +dns_zone_getcfg(dns_zone_t *zone) { + REQUIRE(DNS_ZONE_VALID(zone)); + + return zone->cfg; +} From d03f6e6fd47ff083f9b3bfa17306d7de00b517d2 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Wed, 22 Oct 2025 11:49:09 +0200 Subject: [PATCH 07/12] cfg_parse_ functions internally handle the parser Instead of (1) allocating a parser, (2) parsing a file/buffer then (3) freeing the parser, the parser is now internally created/destroyed from within the `cfg_parse_*` functions. This simplifies a lot the use cases, especially around the error cases where the parser needs to be freed in a cleanup goto. The only trick was the parser callback mechanism, which would previously have been set up between steps 1 and 2. Since it's never been used for any purpose other than the "directory" option, the chdir call has now been moved inside the parser and the generic callback mechanism has been removed, replacing CFG_CLAUSEFLAG_CALLBACK with CFG_CLAUSEFLAG_CHDIR. --- bin/check/named-checkconf.c | 52 +----- bin/delv/delv.c | 13 +- bin/dig/dighost.c | 16 +- bin/dnssec/dnssec-keygen.c | 8 +- bin/dnssec/dnssec-ksr.c | 5 +- bin/named/config.c | 77 +-------- bin/named/controlconf.c | 7 +- bin/named/include/named/config.h | 2 +- bin/named/server.c | 93 +++------- bin/nsupdate/nsupdate.c | 15 +- bin/plugins/filter-a.c | 16 +- bin/plugins/filter-aaaa.c | 16 +- bin/plugins/synthrecord.c | 9 +- bin/rndc/rndc.c | 11 +- bin/tests/system/checkconf/tests.sh | 1 + .../system/hooks/driver/test-syncplugin.c | 9 +- doc/misc/cfg_test.c | 7 +- lib/isccfg/include/isccfg/cfg.h | 30 +--- lib/isccfg/include/isccfg/grammar.h | 13 +- lib/isccfg/namedconf.c | 2 +- lib/isccfg/parser.c | 162 ++++++++++++------ tests/isccfg/duration_test.c | 8 +- tests/isccfg/grammar_test.c | 10 +- tests/isccfg/meson.build | 1 + tests/isccfg/parser_test.c | 92 ++++++---- 25 files changed, 250 insertions(+), 425 deletions(-) diff --git a/bin/check/named-checkconf.c b/bin/check/named-checkconf.c index 0c217efd92..169fd66ef7 100644 --- a/bin/check/named-checkconf.c +++ b/bin/check/named-checkconf.c @@ -63,32 +63,6 @@ usage(void) { exit(EXIT_SUCCESS); } -/*% directory callback */ -static isc_result_t -directory_callback(const char *clausename, const cfg_obj_t *obj, void *arg) { - isc_result_t result; - const char *directory; - - REQUIRE(strcasecmp("directory", clausename) == 0); - - UNUSED(arg); - UNUSED(clausename); - - /* - * Change directory. - */ - directory = cfg_obj_asstring(obj); - result = isc_dir_chdir(directory); - if (result != ISC_R_SUCCESS) { - cfg_obj_log(obj, ISC_LOG_ERROR, - "change directory to '%s' failed: %s\n", directory, - isc_result_totext(result)); - return result; - } - - return ISC_R_SUCCESS; -} - static bool get_maps(const cfg_obj_t **maps, const char *name, const cfg_obj_t **obj) { int i; @@ -574,16 +548,14 @@ output(void *closure, const char *text, int textlen) { int main(int argc, char **argv) { int c; - cfg_parser_t *parser = NULL; cfg_obj_t *config = NULL; const char *conffile = NULL; isc_result_t result = ISC_R_SUCCESS; bool load_zones = false; bool list_zones = false; bool print = false; - bool nodeprecate = false; - bool allconfigs = false; unsigned int flags = 0; + unsigned int parserflags = 0; unsigned int checkflags = BIND_CHECK_PLUGINS | BIND_CHECK_ALGORITHMS; isc_commandline_init(argc, argv); @@ -631,7 +603,7 @@ main(int argc, char **argv) { break; case 'i': - nodeprecate = true; + parserflags |= CFG_PCTX_NODEPRECATED; break; case 'j': @@ -650,7 +622,7 @@ main(int argc, char **argv) { break; case 'n': - allconfigs = true; + parserflags |= CFG_PCTX_ALLCONFIGS; break; case 't': @@ -722,18 +694,8 @@ main(int argc, char **argv) { } CHECK(setup_logging(stdout)); - - CHECK(cfg_parser_create(isc_g_mctx, &parser)); - - if (nodeprecate) { - cfg_parser_setflags(parser, CFG_PCTX_NODEPRECATED, true); - } - if (allconfigs) { - cfg_parser_setflags(parser, CFG_PCTX_ALLCONFIGS, true); - } - cfg_parser_setcallback(parser, directory_callback, NULL); - - CHECK(cfg_parse_file(parser, conffile, &cfg_type_namedconf, &config)); + CHECK(cfg_parse_file(isc_g_mctx, conffile, &cfg_type_namedconf, + parserflags, &config)); CHECK(isccfg_check_namedconf(config, checkflags, isc_g_mctx)); if (load_zones || list_zones) { CHECK(load_zones_fromconfig(config, list_zones)); @@ -748,9 +710,5 @@ cleanup: cfg_obj_detach(&config); } - if (parser != NULL) { - cfg_parser_destroy(&parser); - } - return result == ISC_R_SUCCESS ? 0 : 1; } diff --git a/bin/delv/delv.c b/bin/delv/delv.c index dd53c825fa..b05e05f2d2 100644 --- a/bin/delv/delv.c +++ b/bin/delv/delv.c @@ -808,7 +808,6 @@ cleanup: static isc_result_t setup_dnsseckeys(dns_client_t *client, dns_view_t *toview) { isc_result_t result; - cfg_parser_t *parser = NULL; const cfg_obj_t *trust_anchors = NULL; cfg_obj_t *bindkeys = NULL; @@ -824,15 +823,13 @@ setup_dnsseckeys(dns_client_t *client, dns_view_t *toview) { CHECK(convert_name(&afn, &anchor_name, trust_anchor)); } - CHECK(cfg_parser_create(isc_g_mctx, &parser)); - if (anchorfile != NULL) { if (access(anchorfile, R_OK) != 0) { fatal("Unable to read key file '%s'", anchorfile); } - result = cfg_parse_file(parser, anchorfile, &cfg_type_bindkeys, - &bindkeys); + result = cfg_parse_file(isc_g_mctx, anchorfile, + &cfg_type_bindkeys, 0, &bindkeys); if (result != ISC_R_SUCCESS) { fatal("Unable to load keys from '%s'", anchorfile); } @@ -841,8 +838,7 @@ setup_dnsseckeys(dns_client_t *client, dns_view_t *toview) { isc_buffer_init(&b, anchortext, sizeof(anchortext) - 1); isc_buffer_add(&b, sizeof(anchortext) - 1); - cfg_parser_reset(parser); - result = cfg_parse_buffer(parser, &b, NULL, 0, + result = cfg_parse_buffer(isc_g_mctx, &b, NULL, 0, &cfg_type_bindkeys, 0, &bindkeys); if (result != ISC_R_SUCCESS) { fatal("Unable to parse built-in keys"); @@ -864,9 +860,6 @@ cleanup: if (bindkeys != NULL) { cfg_obj_detach(&bindkeys); } - if (parser != NULL) { - cfg_parser_destroy(&parser); - } if (result != ISC_R_SUCCESS) { delv_log(ISC_LOG_ERROR, "setup_dnsseckeys: %s", isc_result_totext(result)); diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 51949c2cc9..5928c265bd 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -1072,7 +1072,6 @@ parse_hmac(const char *algname) { */ static isc_result_t read_confkey(void) { - cfg_parser_t *pctx = NULL; cfg_obj_t *file = NULL; const cfg_obj_t *keyobj = NULL; const cfg_obj_t *secretobj = NULL; @@ -1086,12 +1085,8 @@ read_confkey(void) { return ISC_R_FILENOTFOUND; } - result = cfg_parser_create(isc_g_mctx, &pctx); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } - - result = cfg_parse_file(pctx, keyfile, &cfg_type_sessionkey, &file); + result = cfg_parse_file(isc_g_mctx, keyfile, &cfg_type_sessionkey, 0, + &file); if (result != ISC_R_SUCCESS) { goto cleanup; } @@ -1117,11 +1112,8 @@ read_confkey(void) { setup_text_key(); cleanup: - if (pctx != NULL) { - if (file != NULL) { - cfg_obj_detach(&file); - } - cfg_parser_destroy(&pctx); + if (file != NULL) { + cfg_obj_detach(&file); } return result; diff --git a/bin/dnssec/dnssec-keygen.c b/bin/dnssec/dnssec-keygen.c index ab53148b44..6c6e82f097 100644 --- a/bin/dnssec/dnssec-keygen.c +++ b/bin/dnssec/dnssec-keygen.c @@ -1147,14 +1147,11 @@ main(int argc, char **argv) { keygen(&ctx, argc, argv); } else { - cfg_parser_t *parser = NULL; cfg_obj_t *config = NULL; dns_kasp_t *kasp = NULL; - RUNTIME_CHECK(cfg_parser_create(isc_g_mctx, &parser) == - ISC_R_SUCCESS); - if (cfg_parse_file(parser, ctx.configfile, - &cfg_type_namedconf, + if (cfg_parse_file(isc_g_mctx, ctx.configfile, + &cfg_type_namedconf, 0, &config) != ISC_R_SUCCESS) { fatal("unable to load dnssec-policy '%s' from " @@ -1200,7 +1197,6 @@ main(int argc, char **argv) { dns_kasp_detach(&kasp); cfg_obj_detach(&config); - cfg_parser_destroy(&parser); } } else { keygen(&ctx, argc, argv); diff --git a/bin/dnssec/dnssec-ksr.c b/bin/dnssec/dnssec-ksr.c index 0a43fd889a..a4ef81b82e 100644 --- a/bin/dnssec/dnssec-ksr.c +++ b/bin/dnssec/dnssec-ksr.c @@ -170,11 +170,9 @@ checkparams(ksr_ctx_t *ksr, const char *command) { static void getkasp(ksr_ctx_t *ksr, dns_kasp_t **kasp) { - cfg_parser_t *parser = NULL; cfg_obj_t *config = NULL; - RUNTIME_CHECK(cfg_parser_create(isc_g_mctx, &parser) == ISC_R_SUCCESS); - if (cfg_parse_file(parser, ksr->configfile, &cfg_type_namedconf, + if (cfg_parse_file(isc_g_mctx, ksr->configfile, &cfg_type_namedconf, 0, &config) != ISC_R_SUCCESS) { fatal("unable to load dnssec-policy '%s' from '%s'", @@ -188,7 +186,6 @@ getkasp(ksr_ctx_t *ksr, dns_kasp_t **kasp) { fatal("dnssec-policy '%s' has no keys configured", ksr->policy); } cfg_obj_detach(&config); - cfg_parser_destroy(&parser); } static int diff --git a/bin/named/config.c b/bin/named/config.c index d7022be01b..00006a8522 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -363,89 +363,28 @@ remote-servers " DEFAULT_IANA_ROOT_ZONE_PRIMARIES " {\n\ isc_result_t named_config_parsedefaults(cfg_obj_t **conf) { isc_buffer_t b; - cfg_parser_t *parser = NULL; - isc_result_t result; - - result = cfg_parser_create(isc_g_mctx, &parser); - if (result != ISC_R_SUCCESS) { - return result; - } isc_buffer_init(&b, defaultconf, sizeof(defaultconf) - 1); isc_buffer_add(&b, sizeof(defaultconf) - 1); - result = cfg_parse_buffer(parser, &b, __FILE__, 0, &cfg_type_namedconf, - CFG_PCTX_NODEPRECATED | CFG_PCTX_NOOBSOLETE | - CFG_PCTX_NOEXPERIMENTAL, - conf); - - cfg_parser_destroy(&parser); - return result; -} - -/* - * This function is called as soon as the 'directory' statement has been - * parsed. This can be extended to support other options if necessary. - */ -static isc_result_t -directory_callback(const char *clausename, const cfg_obj_t *obj, void *arg) { - isc_result_t result; - const char *directory; - - REQUIRE(strcasecmp("directory", clausename) == 0); - - UNUSED(arg); - UNUSED(clausename); - - /* - * Change directory. - */ - directory = cfg_obj_asstring(obj); - - if (!isc_file_ischdiridempotent(directory)) { - cfg_obj_log(obj, ISC_LOG_WARNING, - "option 'directory' contains relative path '%s'", - directory); - } - - if (!isc_file_isdirwritable(directory)) { - isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, - ISC_LOG_ERROR, "directory '%s' is not writable", - directory); - return ISC_R_NOPERM; - } - - result = isc_dir_chdir(directory); - if (result != ISC_R_SUCCESS) { - cfg_obj_log(obj, ISC_LOG_ERROR, - "change directory to '%s' failed: %s", directory, - isc_result_totext(result)); - return result; - } - - char cwd[PATH_MAX]; - if (getcwd(cwd, sizeof(cwd)) == cwd) { - isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, - ISC_LOG_INFO, "the working directory is now '%s'", - cwd); - } - - return ISC_R_SUCCESS; + return cfg_parse_buffer(isc_g_mctx, &b, __FILE__, 0, + &cfg_type_namedconf, + CFG_PCTX_NODEPRECATED | CFG_PCTX_NOOBSOLETE | + CFG_PCTX_NOEXPERIMENTAL, + conf); } isc_result_t -named_config_parsefile(cfg_parser_t *parser, cfg_obj_t **conf) { +named_config_parsefile(cfg_obj_t **conf) { isc_result_t result; - REQUIRE(parser); REQUIRE(conf && *conf == NULL); isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, "parsing user configuration from '%s'", named_g_conffile); - cfg_parser_setcallback(parser, directory_callback, NULL); - result = cfg_parse_file(parser, named_g_conffile, &cfg_type_namedconf, - conf); + result = cfg_parse_file(isc_g_mctx, named_g_conffile, + &cfg_type_namedconf, 0, conf); if (result != ISC_R_SUCCESS) { goto cleanup; } diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c index 0ae02c8891..0376d2b770 100644 --- a/bin/named/controlconf.c +++ b/bin/named/controlconf.c @@ -787,7 +787,6 @@ register_keys(const cfg_obj_t *control, const cfg_obj_t *keylist, static isc_result_t get_rndckey(isc_mem_t *mctx, controlkeylist_t *keyids) { isc_result_t result; - cfg_parser_t *pctx = NULL; cfg_obj_t *config = NULL; const cfg_obj_t *key = NULL; const cfg_obj_t *algobj = NULL; @@ -806,8 +805,7 @@ get_rndckey(isc_mem_t *mctx, controlkeylist_t *keyids) { return ISC_R_FILENOTFOUND; } - CHECK(cfg_parser_create(mctx, &pctx)); - CHECK(cfg_parse_file(pctx, named_g_keyfile, &cfg_type_rndckey, + CHECK(cfg_parse_file(mctx, named_g_keyfile, &cfg_type_rndckey, 0, &config)); CHECK(cfg_map_get(config, "key", &key)); @@ -863,9 +861,6 @@ cleanup: if (config != NULL) { cfg_obj_detach(&config); } - if (pctx != NULL) { - cfg_parser_destroy(&pctx); - } return result; } diff --git a/bin/named/include/named/config.h b/bin/named/include/named/config.h index 6b275727d4..3de19d73f8 100644 --- a/bin/named/include/named/config.h +++ b/bin/named/include/named/config.h @@ -28,7 +28,7 @@ isc_result_t named_config_parsedefaults(cfg_obj_t **conf); isc_result_t -named_config_parsefile(cfg_parser_t *parser, cfg_obj_t **conf); +named_config_parsefile(cfg_obj_t **conf); const char * named_config_getdefault(void); diff --git a/bin/named/server.c b/bin/named/server.c index 5fdfedd082..989bcee49c 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -2349,7 +2349,6 @@ catz_addmodzone_cb(void *arg) { cfg_obj_t *zoneobj = NULL; ns_cfgctx_t *cfg = NULL; dns_zone_t *zone = NULL; - cfg_parser_t *parser = NULL; if (isc_loop_shuttingdown(isc_loop_get(isc_tid()))) { goto cleanup; @@ -2464,10 +2463,8 @@ catz_addmodzone_cb(void *arg) { confbuf = NULL; result = dns_catz_generate_zonecfg(cz->origin, cz->entry, &confbuf); if (result == ISC_R_SUCCESS) { - CHECK(cfg_parser_create(cfg->mctx, &parser)); - result = cfg_parse_buffer(parser, confbuf, "catz", 0, + result = cfg_parse_buffer(cfg->mctx, confbuf, "catz", 0, &cfg_type_addzoneconf, 0, &zoneconf); - cfg_parser_destroy(&parser); isc_buffer_free(&confbuf); } /* @@ -2721,7 +2718,6 @@ catz_reconfigure(dns_catz_entry_t *entry, void *arg1, void *arg2) { ns_cfgctx_t *cfg = NULL; dns_zone_t *zone = NULL; isc_result_t result; - cfg_parser_t *parser = NULL; isc_buffer_init(&namebuf, nameb, DNS_NAME_FORMATSIZE); dns_name_totext(dns_catz_entry_getname(entry), DNS_NAME_OMITFINALDOT, @@ -2745,10 +2741,8 @@ catz_reconfigure(dns_catz_entry_t *entry, void *arg1, void *arg2) { result = dns_catz_generate_zonecfg(data->catz, entry, &confbuf); if (result == ISC_R_SUCCESS) { - CHECK(cfg_parser_create(cfg->mctx, &parser)); - result = cfg_parse_buffer(parser, confbuf, "catz", 0, + result = cfg_parse_buffer(cfg->mctx, confbuf, "catz", 0, &cfg_type_addzoneconf, 0, &zoneconf); - cfg_parser_destroy(&parser); isc_buffer_free(&confbuf); } /* @@ -7558,7 +7552,6 @@ data_to_cfg(dns_view_t *view, MDB_val *key, MDB_val *data, isc_buffer_t **text, size_t zone_config_len; cfg_obj_t *zoneconf = NULL; char bufname[DNS_NAME_FORMATSIZE]; - cfg_parser_t *parser = NULL; REQUIRE(view != NULL); REQUIRE(key != NULL); @@ -7596,10 +7589,8 @@ data_to_cfg(dns_view_t *view, MDB_val *key, MDB_val *data, isc_buffer_t **text, snprintf(bufname, sizeof(bufname), "%.*s", (int)zone_name_len, zone_name); - CHECK(cfg_parser_create(view->mctx, &parser)); - result = cfg_parse_buffer(parser, *text, bufname, 0, + result = cfg_parse_buffer(view->mctx, *text, bufname, 0, &cfg_type_addzoneconf, 0, &zoneconf); - cfg_parser_destroy(&parser); if (result != ISC_R_SUCCESS) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, @@ -9232,14 +9223,12 @@ cleanup_aclctx: static isc_result_t load_configuration(named_server_t *server, bool first_time) { isc_result_t result; - cfg_parser_t *parser = NULL; cfg_obj_t *config = NULL, *bindkeys = NULL; isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_DEBUG(1), "load_configuration"); - CHECK(cfg_parser_create(isc_g_mctx, &parser)); - CHECK(named_config_parsefile(parser, &config)); + CHECK(named_config_parsefile(&config)); if (named_g_bindkeysfile != NULL) { /* @@ -9255,9 +9244,9 @@ load_configuration(named_server_t *server, bool first_time) { "keys instead", named_g_bindkeysfile); } else { - cfg_parser_reset(parser); - result = cfg_parse_file(parser, named_g_bindkeysfile, - &cfg_type_bindkeys, &bindkeys); + result = cfg_parse_file( + isc_g_mctx, named_g_bindkeysfile, + &cfg_type_bindkeys, 0, &bindkeys); if (result != ISC_R_SUCCESS) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, @@ -9274,9 +9263,6 @@ load_configuration(named_server_t *server, bool first_time) { result = apply_configuration(config, bindkeys, server, first_time); cleanup: - if (parser != NULL) { - cfg_parser_destroy(&parser); - } if (bindkeys != NULL) { cfg_obj_detach(&bindkeys); } @@ -12466,9 +12452,8 @@ load_nzf(dns_view_t *view, ns_cfgctx_t *nzcfg) { * Parse the configuration in the NZF file. This may be called in * multiple views, so we reset the parser each time. */ - cfg_parser_reset(named_g_addparser); - result = cfg_parse_file(named_g_addparser, view->new_zone_file, - &cfg_type_addzoneconf, &nzcfg->nzf_config); + result = cfg_parse_file(nzcfg->mctx, view->new_zone_file, + &cfg_type_addzoneconf, 0, &nzcfg->nzf_config); if (result != ISC_R_SUCCESS) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, "Error parsing NZF file '%s': %s", @@ -12816,7 +12801,6 @@ load_nzf(dns_view_t *view, ns_cfgctx_t *nzcfg) { MDB_dbi dbi; MDB_val key, data; ns_dzarg_t dzarg; - cfg_parser_t *parser = NULL; UNUSED(nzcfg); @@ -12840,10 +12824,8 @@ load_nzf(dns_view_t *view, ns_cfgctx_t *nzcfg) { * config type, giving us a guarantee that valid configuration * will be written to DB. */ - CHECK(cfg_parser_create(nzcfg->mctx, &parser)); - result = cfg_parse_file(parser, view->new_zone_file, - &cfg_type_addzoneconf, &nzf_config); - cfg_parser_destroy(&parser); + result = cfg_parse_file(nzcfg->mctx, view->new_zone_file, + &cfg_type_addzoneconf, 0, &nzf_config); if (result != ISC_R_SUCCESS) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, "Error parsing NZF file '%s': %s", @@ -12973,7 +12955,6 @@ newzone_parse(named_server_t *server, char *command, dns_view_t **viewp, dns_rdataclass_t rdclass; dns_view_t *view = NULL; const char *bn = NULL; - cfg_parser_t *parser = NULL; REQUIRE(viewp != NULL && *viewp == NULL); REQUIRE(zoneobjp != NULL && *zoneobjp == NULL); @@ -12998,10 +12979,8 @@ newzone_parse(named_server_t *server, char *command, dns_view_t **viewp, */ isc_buffer_forward(&argbuf, 3); - CHECK(cfg_parser_create(server->mctx, &parser)); - CHECK(cfg_parse_buffer(parser, &argbuf, bn, 0, &cfg_type_addzoneconf, 0, - &zoneconf)); - cfg_parser_destroy(&parser); + CHECK(cfg_parse_buffer(server->mctx, &argbuf, bn, 0, + &cfg_type_addzoneconf, 0, &zoneconf)); CHECK(cfg_map_get(zoneconf, "zone", &zlist)); if (!cfg_obj_islist(zlist)) { @@ -13076,10 +13055,6 @@ newzone_parse(named_server_t *server, char *command, dns_view_t **viewp, return ISC_R_SUCCESS; cleanup: - if (parser != NULL) { - cfg_parser_destroy(&parser); - } - if (zoneconf != NULL) { cfg_obj_detach(&zoneconf); } @@ -13091,13 +13066,12 @@ cleanup: } static isc_result_t -delete_zoneconf(dns_view_t *view, cfg_parser_t *pctx, const cfg_obj_t *config, +delete_zoneconf(dns_view_t *view, const cfg_obj_t *config, const dns_name_t *zname, nzfwriter_t nzfwriter) { isc_result_t result = ISC_R_NOTFOUND; const cfg_obj_t *zl = NULL; REQUIRE(view != NULL); - REQUIRE(pctx != NULL); REQUIRE(config != NULL); REQUIRE(zname != NULL); @@ -13126,7 +13100,6 @@ delete_zoneconf(dns_view_t *view, cfg_parser_t *pctx, const cfg_obj_t *config, e = UNCONST(elt); ISC_LIST_UNLINK(*list, e, link); cfg_obj_detach(&e->obj); - isc_mem_put(pctx->mctx, e, sizeof(*e)); result = ISC_R_SUCCESS; break; } @@ -13263,8 +13236,7 @@ do_addzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, cfg_obj_attach(zoneconf, &cfg->nzf_config); } else { cfg_obj_t *z = UNCONST(zoneobj); - CHECK(cfg_parser_mapadd(cfg->add_parser, cfg->nzf_config, z, - "zone")); + CHECK(cfg_parser_mapadd(cfg->nzf_config, z, "zone")); } cleanup_config = true; #endif /* HAVE_LMDB */ @@ -13313,8 +13285,7 @@ cleanup: (void)isc_stdio_close(fp); } if (result != ISC_R_SUCCESS && cleanup_config) { - tresult = delete_zoneconf(view, cfg->add_parser, - cfg->nzf_config, name, NULL); + tresult = delete_zoneconf(view, cfg->nzf_config, name, NULL); RUNTIME_CHECK(tresult == ISC_R_SUCCESS); } #else /* HAVE_LMDB */ @@ -13341,7 +13312,6 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, dns_zone_t *zone = NULL; const cfg_obj_t *voptions = NULL; bool added; - cfg_parser_t *parser = NULL; #ifndef HAVE_LMDB FILE *fp = NULL; cfg_obj_t *z; @@ -13448,7 +13418,7 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, #ifndef HAVE_LMDB /* Remove old zone from configuration (and NZF file if applicable) */ if (added) { - result = delete_zoneconf(view, cfg->add_parser, cfg->nzf_config, + result = delete_zoneconf(view, cfg->nzf_config, dns_zone_getorigin(zone), nzf_writeconf); if (result != ISC_R_SUCCESS) { @@ -13461,18 +13431,15 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, #endif /* HAVE_LMDB */ if (!added) { - TCHECK(cfg_parser_create(cfg->mctx, &parser)); if (cfg->vconfig == NULL) { - result = delete_zoneconf(view, parser, cfg->config, + result = delete_zoneconf(view, cfg->config, dns_zone_getorigin(zone), NULL); } else { voptions = cfg_tuple_get(cfg->vconfig, "options"); - result = delete_zoneconf(view, parser, voptions, - dns_zone_getorigin(zone), - NULL); + result = delete_zoneconf( + view, voptions, dns_zone_getorigin(zone), NULL); } - cfg_parser_destroy(&parser); if (result != ISC_R_SUCCESS) { TCHECK(putstr(text, "former zone configuration " @@ -13520,7 +13487,7 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, #ifndef HAVE_LMDB /* Store the new zone configuration; also in NZF if applicable */ z = UNCONST(zoneobj); - CHECK(cfg_parser_mapadd(cfg->add_parser, cfg->nzf_config, z, "zone")); + CHECK(cfg_parser_mapadd(cfg->nzf_config, z, "zone")); #endif /* HAVE_LMDB */ if (added) { @@ -13695,7 +13662,6 @@ rmzone(void *arg) { dns_db_t *dbp = NULL; bool added; isc_result_t result; - cfg_parser_t *parser = NULL; #ifdef HAVE_LMDB MDB_txn *txn = NULL; MDB_dbi dbi; @@ -13746,7 +13712,7 @@ rmzone(void *arg) { } UNLOCK(&view->new_zone_lock); #else /* ifdef HAVE_LMDB */ - result = delete_zoneconf(view, cfg->add_parser, cfg->nzf_config, + result = delete_zoneconf(view, cfg->nzf_config, dns_zone_getorigin(zone), nzf_writeconf); if (result != ISC_R_SUCCESS) { @@ -13759,24 +13725,17 @@ rmzone(void *arg) { } if (!added && cfg != NULL) { - result = cfg_parser_create(cfg->mctx, &parser); - INSIST(result == ISC_R_SUCCESS); /* colin: this is just a - shortcut until the - initialization function of - the parser can't fails */ - if (cfg->vconfig != NULL) { const cfg_obj_t *voptions = cfg_tuple_get(cfg->vconfig, "options"); - result = delete_zoneconf(view, parser, voptions, - dns_zone_getorigin(zone), - NULL); + result = delete_zoneconf( + view, voptions, dns_zone_getorigin(zone), NULL); } else { - result = delete_zoneconf(view, parser, cfg->config, + result = delete_zoneconf(view, cfg->config, dns_zone_getorigin(zone), NULL); } - cfg_parser_destroy(&parser); + if (result != ISC_R_SUCCESS) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index 585dc80b91..4ceff56466 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -555,7 +555,6 @@ failure: */ static isc_result_t read_sessionkey(isc_mem_t *mctx) { - cfg_parser_t *pctx = NULL; cfg_obj_t *sessionkey = NULL; const cfg_obj_t *key = NULL; const cfg_obj_t *secretobj = NULL; @@ -570,12 +569,7 @@ read_sessionkey(isc_mem_t *mctx) { return ISC_R_FILENOTFOUND; } - result = cfg_parser_create(mctx, &pctx); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } - - result = cfg_parse_file(pctx, keyfile, &cfg_type_sessionkey, + result = cfg_parse_file(mctx, keyfile, &cfg_type_sessionkey, 0, &sessionkey); if (result != ISC_R_SUCCESS) { goto cleanup; @@ -602,11 +596,8 @@ read_sessionkey(isc_mem_t *mctx) { setup_keystr(); cleanup: - if (pctx != NULL) { - if (sessionkey != NULL) { - cfg_obj_detach(&sessionkey); - } - cfg_parser_destroy(&pctx); + if (sessionkey != NULL) { + cfg_obj_detach(&sessionkey); } if (keystr != NULL) { diff --git a/bin/plugins/filter-a.c b/bin/plugins/filter-a.c index 5d1cf06e52..2d98b70692 100644 --- a/bin/plugins/filter-a.c +++ b/bin/plugins/filter-a.c @@ -274,16 +274,13 @@ parse_parameters(filter_instance_t *inst, const char *parameters, const void *cfg, const char *cfg_file, unsigned long cfg_line, isc_mem_t *mctx, void *aclctx) { isc_result_t result = ISC_R_SUCCESS; - cfg_parser_t *parser = NULL; cfg_obj_t *param_obj = NULL; const cfg_obj_t *obj = NULL; isc_buffer_t b; - CHECK(cfg_parser_create(mctx, &parser)); - isc_buffer_constinit(&b, parameters, strlen(parameters)); isc_buffer_add(&b, strlen(parameters)); - CHECK(cfg_parse_buffer(parser, &b, cfg_file, cfg_line, + CHECK(cfg_parse_buffer(mctx, &b, cfg_file, cfg_line, &cfg_type_parameters, 0, ¶m_obj)); CHECK(parse_filter_a_on(param_obj, "filter-a-on-v6", &inst->v6_a)); @@ -302,9 +299,6 @@ cleanup: if (param_obj != NULL) { cfg_obj_detach(¶m_obj); } - if (parser != NULL) { - cfg_parser_destroy(&parser); - } return result; } @@ -368,15 +362,12 @@ plugin_check(const char *parameters, const void *cfg, const char *cfg_file, unsigned long cfg_line, isc_mem_t *mctx, void *aclctx, const ns_pluginctx_t *ctx ISC_ATTR_UNUSED) { isc_result_t result = ISC_R_SUCCESS; - cfg_parser_t *parser = NULL; cfg_obj_t *param_obj = NULL; isc_buffer_t b; - CHECK(cfg_parser_create(mctx, &parser)); - isc_buffer_constinit(&b, parameters, strlen(parameters)); isc_buffer_add(&b, strlen(parameters)); - CHECK(cfg_parse_buffer(parser, &b, cfg_file, cfg_line, + CHECK(cfg_parse_buffer(mctx, &b, cfg_file, cfg_line, &cfg_type_parameters, 0, ¶m_obj)); CHECK(check_syntax(param_obj, cfg, mctx, aclctx)); @@ -385,9 +376,6 @@ cleanup: if (param_obj != NULL) { cfg_obj_detach(¶m_obj); } - if (parser != NULL) { - cfg_parser_destroy(&parser); - } return result; } diff --git a/bin/plugins/filter-aaaa.c b/bin/plugins/filter-aaaa.c index ad2ef52f3a..dad917464c 100644 --- a/bin/plugins/filter-aaaa.c +++ b/bin/plugins/filter-aaaa.c @@ -275,16 +275,13 @@ parse_parameters(filter_instance_t *inst, const char *parameters, const void *cfg, const char *cfg_file, unsigned long cfg_line, isc_mem_t *mctx, void *aclctx) { isc_result_t result = ISC_R_SUCCESS; - cfg_parser_t *parser = NULL; cfg_obj_t *param_obj = NULL; const cfg_obj_t *obj = NULL; isc_buffer_t b; - CHECK(cfg_parser_create(mctx, &parser)); - isc_buffer_constinit(&b, parameters, strlen(parameters)); isc_buffer_add(&b, strlen(parameters)); - CHECK(cfg_parse_buffer(parser, &b, cfg_file, cfg_line, + CHECK(cfg_parse_buffer(mctx, &b, cfg_file, cfg_line, &cfg_type_parameters, 0, ¶m_obj)); CHECK(parse_filter_aaaa_on(param_obj, "filter-aaaa-on-v4", @@ -305,9 +302,6 @@ cleanup: if (param_obj != NULL) { cfg_obj_detach(¶m_obj); } - if (parser != NULL) { - cfg_parser_destroy(&parser); - } return result; } @@ -372,15 +366,12 @@ plugin_check(const char *parameters, const void *cfg, const char *cfg_file, unsigned long cfg_line, isc_mem_t *mctx, void *aclctx, const ns_pluginctx_t *ctx ISC_ATTR_UNUSED) { isc_result_t result = ISC_R_SUCCESS; - cfg_parser_t *parser = NULL; cfg_obj_t *param_obj = NULL; isc_buffer_t b; - CHECK(cfg_parser_create(mctx, &parser)); - isc_buffer_constinit(&b, parameters, strlen(parameters)); isc_buffer_add(&b, strlen(parameters)); - CHECK(cfg_parse_buffer(parser, &b, cfg_file, cfg_line, + CHECK(cfg_parse_buffer(mctx, &b, cfg_file, cfg_line, &cfg_type_parameters, 0, ¶m_obj)); CHECK(check_syntax(param_obj, cfg, mctx, aclctx)); @@ -389,9 +380,6 @@ cleanup: if (param_obj != NULL) { cfg_obj_detach(¶m_obj); } - if (parser != NULL) { - cfg_parser_destroy(&parser); - } return result; } diff --git a/bin/plugins/synthrecord.c b/bin/plugins/synthrecord.c index 5c5f34bf89..1fab2a3a37 100644 --- a/bin/plugins/synthrecord.c +++ b/bin/plugins/synthrecord.c @@ -573,16 +573,13 @@ synthrecord_parseconfig(synthrecord_t *inst, const char *parameters, const dns_name_t *zname) { isc_result_t result; isc_mem_t *mctx = inst->mctx; - cfg_parser_t *parser = NULL; cfg_obj_t *synthrecordcfg = NULL; isc_buffer_t b; - CHECK(cfg_parser_create(mctx, &parser)); - isc_buffer_constinit(&b, parameters, strlen(parameters)); isc_buffer_add(&b, strlen(parameters)); - CHECK(cfg_parse_buffer(parser, &b, cfgfile, cfgline, + CHECK(cfg_parse_buffer(mctx, &b, cfgfile, cfgline, &synthrecord_cfgparams, 0, &synthrecordcfg)); synthrecord_setconfigmode(inst, zname); @@ -596,10 +593,6 @@ cleanup: cfg_obj_detach(&synthrecordcfg); } - if (parser != NULL) { - cfg_parser_destroy(&parser); - } - return result; } diff --git a/bin/rndc/rndc.c b/bin/rndc/rndc.c index 3204e30a63..46d0aa0c97 100644 --- a/bin/rndc/rndc.c +++ b/bin/rndc/rndc.c @@ -535,8 +535,7 @@ rndc_start(void *arg) { } static void -parse_config(isc_mem_t *mctx, const char *keyname, cfg_parser_t **pctxp, - cfg_obj_t **configp) { +parse_config(isc_mem_t *mctx, const char *keyname, cfg_obj_t **configp) { isc_result_t result; const char *conffile = admin_conffile; const cfg_obj_t *addresses = NULL; @@ -577,12 +576,10 @@ parse_config(isc_mem_t *mctx, const char *keyname, cfg_parser_t **pctxp, admin_keyfile, admin_conffile); } - DO("create parser", cfg_parser_create(mctx, pctxp)); - /* * The parser will output its own errors, so DO() is not used. */ - result = cfg_parse_file(*pctxp, conffile, conftype, &config); + result = cfg_parse_file(mctx, conffile, conftype, 0, &config); if (result != ISC_R_SUCCESS) { fatal("could not load rndc configuration"); } @@ -809,7 +806,6 @@ int main(int argc, char **argv) { bool show_final_mem = false; isc_logconfig_t *logconfig = NULL; - cfg_parser_t *pctx = NULL; cfg_obj_t *config = NULL; const char *keyname = NULL; struct in_addr in; @@ -963,7 +959,7 @@ main(int argc, char **argv) { ISC_LOG_PRINTTAG | ISC_LOG_PRINTLEVEL, ISC_LOGCATEGORY_DEFAULT, ISC_LOGMODULE_DEFAULT); - parse_config(isc_g_mctx, keyname, &pctx, &config); + parse_config(isc_g_mctx, keyname, &config); isc_buffer_allocate(isc_g_mctx, &databuf, 2048); @@ -1000,7 +996,6 @@ main(int argc, char **argv) { isccc_ccmsg_invalidate(&rndc_ccmsg); cfg_obj_detach(&config); - cfg_parser_destroy(&pctx); isc_mem_put(isc_g_mctx, args, argslen); diff --git a/bin/tests/system/checkconf/tests.sh b/bin/tests/system/checkconf/tests.sh index efb781188b..aec7b13639 100644 --- a/bin/tests/system/checkconf/tests.sh +++ b/bin/tests/system/checkconf/tests.sh @@ -522,6 +522,7 @@ n=$((n + 1)) echo_i "check that named-checkconf -l prints out the zone list ($n)" ret=0 $CHECKCONF -l good.conf \ + | grep -v "working directory is" \ | grep -v "is deprecated" \ | grep -v "is not implemented" \ | grep -v "is not recommended" \ diff --git a/bin/tests/system/hooks/driver/test-syncplugin.c b/bin/tests/system/hooks/driver/test-syncplugin.c index 09856fd2cc..620829c8cc 100644 --- a/bin/tests/system/hooks/driver/test-syncplugin.c +++ b/bin/tests/system/hooks/driver/test-syncplugin.c @@ -116,7 +116,6 @@ plugin_register(const char *parameters, const void *cfg, const char *cfgfile, ns_hooktable_t *hooktable, const ns_pluginctx_t *ctx, void **instp) { isc_result_t result; - cfg_parser_t *parser = NULL; cfg_obj_t *syncplugincfg = NULL; const cfg_obj_t *obj = NULL; isc_buffer_t b; @@ -135,12 +134,10 @@ plugin_register(const char *parameters, const void *cfg, const char *cfgfile, *inst = (syncplugin_t){ .mctx = mctx }; *instp = inst; - CHECK(cfg_parser_create(mctx, &parser)); - isc_buffer_constinit(&b, parameters, strlen(parameters)); isc_buffer_add(&b, strlen(parameters)); - CHECK(cfg_parse_buffer(parser, &b, cfgfile, cfgline, + CHECK(cfg_parse_buffer(mctx, &b, cfgfile, cfgline, &syncplugin__cfgparams, 0, &syncplugincfg)); CHECK(syncplugin__parse_rcode(syncplugincfg, &inst->rcode)); @@ -220,10 +217,6 @@ cleanup: cfg_obj_detach(&syncplugincfg); } - if (parser != NULL) { - cfg_parser_destroy(&parser); - } - return result; } diff --git a/doc/misc/cfg_test.c b/doc/misc/cfg_test.c index 60b28bc159..f29600c2aa 100644 --- a/doc/misc/cfg_test.c +++ b/doc/misc/cfg_test.c @@ -52,7 +52,6 @@ int main(int argc, char **argv) { isc_result_t result; isc_mem_t *mctx = NULL; - cfg_parser_t *pctx = NULL; cfg_obj_t *cfg = NULL; cfg_type_t *type = NULL; bool grammar = false; @@ -134,9 +133,7 @@ main(int argc, char **argv) { if (type == NULL || filename == NULL) { usage(); } - RUNTIME_CHECK(cfg_parser_create(mctx, &pctx) == ISC_R_SUCCESS); - - result = cfg_parse_file(pctx, filename, type, &cfg); + result = cfg_parse_file(mctx, filename, type, 0, &cfg); fprintf(stderr, "read config: %s\n", isc_result_totext(result)); @@ -147,8 +144,6 @@ main(int argc, char **argv) { cfg_print(cfg, output, NULL); cfg_obj_detach(&cfg); - - cfg_parser_destroy(&pctx); } if (memstats) { diff --git a/lib/isccfg/include/isccfg/cfg.h b/lib/isccfg/include/isccfg/cfg.h index 8e861655a5..b403fa7b28 100644 --- a/lib/isccfg/include/isccfg/cfg.h +++ b/lib/isccfg/include/isccfg/cfg.h @@ -113,25 +113,12 @@ cfg_parser_setflags(cfg_parser_t *pctx, unsigned int flags, bool turn_on); *\li "pctx" is not NULL. */ -void -cfg_parser_setcallback(cfg_parser_t *pctx, cfg_parsecallback_t callback, - void *arg); -/*%< - * Make the parser call 'callback' whenever it encounters - * a configuration clause with the callback attribute, - * passing it the clause name, the clause value, - * and 'arg' as arguments. - * - * To restore the default of not invoking callbacks, pass - * callback==NULL and arg==NULL. - */ +isc_result_t +cfg_parse_file(isc_mem_t *mctx, const char *file, const cfg_type_t *type, + unsigned int flags, cfg_obj_t **ret); isc_result_t -cfg_parse_file(cfg_parser_t *pctx, const char *file, const cfg_type_t *type, - cfg_obj_t **ret); - -isc_result_t -cfg_parse_buffer(cfg_parser_t *pctx, isc_buffer_t *buffer, const char *file, +cfg_parse_buffer(isc_mem_t *mctx, isc_buffer_t *buffer, const char *file, unsigned int line, const cfg_type_t *type, unsigned int flags, cfg_obj_t **ret); /*%< @@ -152,8 +139,9 @@ cfg_parse_buffer(cfg_parser_t *pctx, isc_buffer_t *buffer, const char *file, * Returns an error if the file or buffer does not parse correctly. * * Requires: - *\li "filename" is valid. - *\li "mem" is valid. + *\li "file" is valid. + *\li "buffer" is valid. + *\li "mctx" is valid. *\li "type" is valid. *\li "cfg" is non-NULL and "*cfg" is NULL. *\li "flags" be one or more of CFG_PCTX_NODEPRECATED or zero. @@ -165,8 +153,7 @@ cfg_parse_buffer(cfg_parser_t *pctx, isc_buffer_t *buffer, const char *file, */ isc_result_t -cfg_parser_mapadd(cfg_parser_t *pctx, cfg_obj_t *mapobj, cfg_obj_t *obj, - const char *clause); +cfg_parser_mapadd(cfg_obj_t *mapobj, cfg_obj_t *obj, const char *clause); /*%< * Add the object 'obj' to the specified clause in mapbody 'mapobj'. * Used for adding new zones. @@ -174,7 +161,6 @@ cfg_parser_mapadd(cfg_parser_t *pctx, cfg_obj_t *mapobj, cfg_obj_t *obj, * Require: * \li 'obj' is a valid cfg_obj_t. * \li 'mapobj' is a valid cfg_obj_t of type map. - * \li 'pctx' is a valid cfg_parser_t. */ void diff --git a/lib/isccfg/include/isccfg/grammar.h b/lib/isccfg/include/isccfg/grammar.h index c06fedefb2..8e84e31a37 100644 --- a/lib/isccfg/include/isccfg/grammar.h +++ b/lib/isccfg/include/isccfg/grammar.h @@ -47,11 +47,8 @@ enum { /*% Clause is obsolete (logs a warning, but is not a fatal error) */ CFG_CLAUSEFLAG_OBSOLETE = 1 << 3, - /*% - * Clause needs to be interpreted during parsing by calling a - * callback function, like the "directory" option. - */ - CFG_CLAUSEFLAG_CALLBACK = 1 << 4, + /*% Clause indicates it must change the current directory */ + CFG_CLAUSEFLAG_CHDIR = 1 << 4, /*% Clause that is only used in testing. */ CFG_CLAUSEFLAG_TESTONLY = 1 << 5, @@ -266,9 +263,6 @@ struct cfg_parser { /*%< Reference counter */ isc_refcount_t references; - - cfg_parsecallback_t callback; - void *callbackarg; }; /* Parser context flags */ @@ -438,7 +432,8 @@ void cfg_doc_tuple(cfg_printer_t *pctx, const cfg_type_t *type); isc_result_t -cfg_create_list(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **objp); +cfg_create_list(isc_mem_t *mctx, cfg_obj_t *file, size_t line, + const cfg_type_t *type, cfg_obj_t **objp); isc_result_t cfg_parse_listelt(cfg_parser_t *pctx, const cfg_type_t *elttype, diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c index ce30d4b948..d13fd830f8 100644 --- a/lib/isccfg/namedconf.c +++ b/lib/isccfg/namedconf.c @@ -1232,7 +1232,7 @@ static cfg_clausedef_t options_clauses[] = { { "coresize", NULL, CFG_CLAUSEFLAG_ANCIENT }, { "datasize", NULL, CFG_CLAUSEFLAG_ANCIENT }, { "deallocate-on-exit", NULL, CFG_CLAUSEFLAG_ANCIENT }, - { "directory", &cfg_type_qstring, CFG_CLAUSEFLAG_CALLBACK }, + { "directory", &cfg_type_qstring, CFG_CLAUSEFLAG_CHDIR }, { "dnsrps-library", &cfg_type_qstring, CFG_CLAUSEFLAG_OBSOLETE }, #ifdef HAVE_DNSTAP { "dnstap-output", &cfg_type_dnstapoutput, CFG_CLAUSEFLAG_OPTIONAL }, diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index 822ccc83a2..b2667a2629 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -50,6 +50,7 @@ #include #include #include +#include #include #include #include @@ -114,7 +115,7 @@ static void free_list(cfg_obj_t *obj); static isc_result_t -create_listelt(cfg_parser_t *pctx, cfg_listelt_t **eltp); +create_listelt(isc_mem_t *mctx, cfg_listelt_t **eltp); static isc_result_t create_string(cfg_parser_t *pctx, const char *contents, const cfg_type_t *type, @@ -542,8 +543,6 @@ cfg_parser_create(isc_mem_t *mctx, cfg_parser_t **ret) { pctx->open_files = NULL; pctx->closed_files = NULL; pctx->line = 0; - pctx->callback = NULL; - pctx->callbackarg = NULL; pctx->token.type = isc_tokentype_unknown; pctx->flags = 0; pctx->buf_name = NULL; @@ -563,8 +562,12 @@ cfg_parser_create(isc_mem_t *mctx, cfg_parser_t **ret) { ISC_LEXCOMMENT_CPLUSPLUS | ISC_LEXCOMMENT_SHELL); - CHECK(cfg_create_list(pctx, &cfg_type_filelist, &pctx->open_files)); - CHECK(cfg_create_list(pctx, &cfg_type_filelist, &pctx->closed_files)); + CHECK(cfg_create_list(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_filelist, + &pctx->open_files)); + CHECK(cfg_create_list(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_filelist, + &pctx->closed_files)); *ret = pctx; return ISC_R_SUCCESS; @@ -604,7 +607,7 @@ parser_openfile(cfg_parser_t *pctx, const char *filename) { } CHECK(create_string(pctx, filename, &cfg_type_qstring, &stringobj)); - CHECK(create_listelt(pctx, &elt)); + CHECK(create_listelt(pctx->mctx, &elt)); elt->obj = stringobj; ISC_LIST_APPEND(pctx->open_files->value.list, elt, link); @@ -614,15 +617,6 @@ cleanup: return result; } -void -cfg_parser_setcallback(cfg_parser_t *pctx, cfg_parsecallback_t callback, - void *arg) { - REQUIRE(pctx != NULL); - - pctx->callback = callback; - pctx->callbackarg = arg; -} - void cfg_parser_reset(cfg_parser_t *pctx) { REQUIRE(pctx != NULL); @@ -674,16 +668,25 @@ cleanup: return result; } +#define REQUIRE_PCTX_FLAGS(flags) \ + REQUIRE((flags & ~(CFG_PCTX_NODEPRECATED | CFG_PCTX_NOOBSOLETE | \ + CFG_PCTX_NOEXPERIMENTAL)) == 0) + isc_result_t -cfg_parse_file(cfg_parser_t *pctx, const char *filename, const cfg_type_t *type, - cfg_obj_t **ret) { +cfg_parse_file(isc_mem_t *mctx, const char *filename, const cfg_type_t *type, + unsigned int flags, cfg_obj_t **ret) { isc_result_t result; cfg_listelt_t *elt; + cfg_parser_t *pctx = NULL; - REQUIRE(pctx != NULL); + REQUIRE(mctx != NULL); REQUIRE(filename != NULL); REQUIRE(type != NULL); REQUIRE(ret != NULL && *ret == NULL); + REQUIRE_PCTX_FLAGS(flags); + + CHECK(cfg_parser_create(mctx, &pctx)); + pctx->flags = flags; CHECK(parser_openfile(pctx, filename)); @@ -696,22 +699,27 @@ cfg_parse_file(cfg_parser_t *pctx, const char *filename, const cfg_type_t *type, ISC_LIST_APPEND(pctx->closed_files->value.list, elt, link); cleanup: + if (pctx != NULL) { + cfg_parser_destroy(&pctx); + } + return result; } isc_result_t -cfg_parse_buffer(cfg_parser_t *pctx, isc_buffer_t *buffer, const char *file, +cfg_parse_buffer(isc_mem_t *mctx, isc_buffer_t *buffer, const char *file, unsigned int line, const cfg_type_t *type, unsigned int flags, cfg_obj_t **ret) { isc_result_t result; + cfg_parser_t *pctx = NULL; - REQUIRE(pctx != NULL); + REQUIRE(mctx != NULL); REQUIRE(type != NULL); REQUIRE(buffer != NULL); REQUIRE(ret != NULL && *ret == NULL); - REQUIRE((flags & ~(CFG_PCTX_NODEPRECATED | CFG_PCTX_NOOBSOLETE | - CFG_PCTX_NOEXPERIMENTAL)) == 0); + REQUIRE_PCTX_FLAGS(flags); + CHECK(cfg_parser_create(mctx, &pctx)); CHECK(isc_lex_openbuffer(pctx->lexer, buffer)); pctx->buf_name = file; @@ -725,6 +733,10 @@ cfg_parse_buffer(cfg_parser_t *pctx, isc_buffer_t *buffer, const char *file, pctx->buf_name = NULL; cleanup: + if (pctx != NULL) { + cfg_parser_destroy(&pctx); + } + return result; } @@ -1945,25 +1957,25 @@ cfg_type_t cfg_type_boolean = { "boolean", cfg_parse_boolean, */ isc_result_t -cfg_create_list(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **obj) { +cfg_create_list(isc_mem_t *mctx, cfg_obj_t *file, size_t line, + const cfg_type_t *type, cfg_obj_t **obj) { isc_result_t result; - REQUIRE(pctx != NULL); + REQUIRE(mctx != NULL); REQUIRE(type != NULL); REQUIRE(obj != NULL && *obj == NULL); - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, type, obj)); + CHECK(cfg_create_obj(mctx, file, line, type, obj)); ISC_LIST_INIT((*obj)->value.list); cleanup: return result; } static isc_result_t -create_listelt(cfg_parser_t *pctx, cfg_listelt_t **eltp) { +create_listelt(isc_mem_t *mctx, cfg_listelt_t **eltp) { cfg_listelt_t *elt; - elt = isc_mem_get(pctx->mctx, sizeof(*elt)); + elt = isc_mem_get(mctx, sizeof(*elt)); elt->obj = NULL; ISC_LINK_INIT(elt, link); *eltp = elt; @@ -1996,7 +2008,7 @@ cfg_parse_listelt(cfg_parser_t *pctx, const cfg_type_t *elttype, REQUIRE(elttype != NULL); REQUIRE(ret != NULL && *ret == NULL); - CHECK(create_listelt(pctx, &elt)); + CHECK(create_listelt(pctx->mctx, &elt)); result = cfg_parse_obj(pctx, elttype, &value); if (result != ISC_R_SUCCESS) { @@ -2024,7 +2036,8 @@ parse_list(cfg_parser_t *pctx, const cfg_type_t *listtype, cfg_obj_t **ret) { isc_result_t result; cfg_listelt_t *elt = NULL; - CHECK(cfg_create_list(pctx, listtype, &listobj)); + CHECK(cfg_create_list(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, listtype, &listobj)); for (;;) { CHECK(cfg_peektoken(pctx, 0)); @@ -2119,7 +2132,8 @@ cfg_parse_spacelist(cfg_parser_t *pctx, const cfg_type_t *listtype, listof = listtype->of; - CHECK(cfg_create_list(pctx, listtype, &listobj)); + CHECK(cfg_create_list(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, listtype, &listobj)); for (;;) { cfg_listelt_t *elt = NULL; @@ -2386,8 +2400,9 @@ cfg_parse_mapbody(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { /* Multivalued clause */ cfg_obj_t *listobj = NULL; - CHECK(cfg_create_list(pctx, &cfg_type_implicitlist, - &listobj)); + CHECK(cfg_create_list( + pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_implicitlist, &listobj)); symval.as_pointer = listobj; result = isc_symtab_define_and_return( obj->value.map.symtab, clause->name, @@ -2406,12 +2421,12 @@ cfg_parse_mapbody(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { ISC_LIST_APPEND(listobj->value.list, elt, link); } else { /* Single-valued clause */ - bool callback = ((clause->flags & - CFG_CLAUSEFLAG_CALLBACK) != 0); + bool chdir = ((clause->flags & CFG_CLAUSEFLAG_CHDIR) != + 0); - result = parse_symtab_elt( - pctx, clause->name, clause->type, - obj->value.map.symtab, callback); + result = parse_symtab_elt(pctx, clause->name, + clause->type, + obj->value.map.symtab, chdir); if (result == ISC_R_EXISTS) { cfg_parser_error(pctx, CFG_LOG_NEAR, "'%s' redefined", @@ -2437,17 +2452,60 @@ cleanup: return result; } +static isc_result_t +change_directory(const char *clausename, const cfg_obj_t *obj) { + isc_result_t result; + const char *directory; + + REQUIRE(strcasecmp("directory", clausename) == 0); + + UNUSED(clausename); + + /* + * Change directory. + */ + directory = cfg_obj_asstring(obj); + + if (!isc_file_ischdiridempotent(directory)) { + cfg_obj_log(obj, ISC_LOG_WARNING, + "option 'directory' contains relative path '%s'", + directory); + } + + if (!isc_file_isdirwritable(directory)) { + cfg_obj_log(obj, ISC_LOG_ERROR, + "directory '%s' is not writable", directory); + return ISC_R_NOPERM; + } + + result = isc_dir_chdir(directory); + if (result != ISC_R_SUCCESS) { + cfg_obj_log(obj, ISC_LOG_ERROR, + "change directory to '%s' failed: %s", directory, + isc_result_totext(result)); + return result; + } + + char cwd[PATH_MAX]; + if (getcwd(cwd, sizeof(cwd)) == cwd) { + cfg_obj_log(obj, ISC_LOG_INFO, + "the working directory is now '%s'", cwd); + } + + return ISC_R_SUCCESS; +} + static isc_result_t parse_symtab_elt(cfg_parser_t *pctx, const char *name, cfg_type_t *elttype, - isc_symtab_t *symtab, bool callback) { + isc_symtab_t *symtab, bool chdir) { isc_result_t result; cfg_obj_t *obj = NULL; isc_symvalue_t symval; CHECK(cfg_parse_obj(pctx, elttype, &obj)); - if (callback && pctx->callback != NULL) { - CHECK(pctx->callback(name, obj, pctx->callbackarg)); + if (chdir) { + CHECK(change_directory(name, obj)); } symval.as_pointer = obj; @@ -2857,7 +2915,8 @@ parse_unsupported(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { isc_result_t result; int braces = 0; - CHECK(cfg_create_list(pctx, type, &listobj)); + CHECK(cfg_create_list(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, type, &listobj)); for (;;) { cfg_listelt_t *elt = NULL; @@ -3868,17 +3927,15 @@ cfg_print_grammar(const cfg_type_t *type, unsigned int flags, } isc_result_t -cfg_parser_mapadd(cfg_parser_t *pctx, cfg_obj_t *mapobj, cfg_obj_t *obj, - const char *clausename) { +cfg_parser_mapadd(cfg_obj_t *mapobj, cfg_obj_t *obj, const char *clausename) { isc_result_t result = ISC_R_SUCCESS; - const cfg_map_t *map; + const cfg_map_t *map = NULL; isc_symvalue_t symval; cfg_obj_t *destobj = NULL; cfg_listelt_t *elt = NULL; - const cfg_clausedef_t *const *clauseset; - const cfg_clausedef_t *clause; + const cfg_clausedef_t *const *clauseset = NULL; + const cfg_clausedef_t *clause = NULL; - REQUIRE(pctx != NULL); REQUIRE(mapobj != NULL && mapobj->type->rep == &cfg_rep_map); REQUIRE(obj != NULL); REQUIRE(clausename != NULL); @@ -3903,9 +3960,10 @@ breakout: &symval); if (result == ISC_R_NOTFOUND) { if ((clause->flags & CFG_CLAUSEFLAG_MULTI) != 0) { - CHECK(cfg_create_list(pctx, &cfg_type_implicitlist, + CHECK(cfg_create_list(mapobj->mctx, obj->file, + obj->line, &cfg_type_implicitlist, &destobj)); - CHECK(create_listelt(pctx, &elt)); + CHECK(create_listelt(mapobj->mctx, &elt)); cfg_obj_attach(obj, &elt->obj); ISC_LIST_APPEND(destobj->value.list, elt, link); symval.as_pointer = destobj; @@ -3923,7 +3981,7 @@ breakout: INSIST(result == ISC_R_SUCCESS); if (destobj2->type == &cfg_type_implicitlist) { - CHECK(create_listelt(pctx, &elt)); + CHECK(create_listelt(mapobj->mctx, &elt)); cfg_obj_attach(obj, &elt->obj); ISC_LIST_APPEND(destobj2->value.list, elt, link); } else { @@ -3936,7 +3994,7 @@ breakout: cleanup: if (elt != NULL) { - free_listelt(pctx->mctx, elt); + free_listelt(mapobj->mctx, elt); } CLEANUP_OBJ(destobj); diff --git a/tests/isccfg/duration_test.c b/tests/isccfg/duration_test.c index 4454933a1c..a9cb1fd021 100644 --- a/tests/isccfg/duration_test.c +++ b/tests/isccfg/duration_test.c @@ -120,7 +120,6 @@ ISC_RUN_TEST_IMPL(duration) { "T99999999999H99999999999M99999999999S" }, }; isc_buffer_t buf1; - cfg_parser_t *p1 = NULL; cfg_obj_t *c1 = NULL; bool must_fail = false; @@ -144,14 +143,10 @@ ISC_RUN_TEST_IMPL(duration) { isc_buffer_add(&buf1, strlen(conf) - 1); /* Parse with default line numbering */ - result = cfg_parser_create(isc_g_mctx, &p1); - assert_int_equal(result, ISC_R_SUCCESS); - - result = cfg_parse_buffer(p1, &buf1, "text1", 0, + result = cfg_parse_buffer(isc_g_mctx, &buf1, "text1", 0, &cfg_type_namedconf, 0, &c1); if (must_fail) { assert_int_equal(result, DNS_R_BADTTL); - cfg_parser_destroy(&p1); continue; } assert_int_equal(result, ISC_R_SUCCESS); @@ -193,7 +188,6 @@ ISC_RUN_TEST_IMPL(duration) { } cfg_obj_detach(&c1); - cfg_parser_destroy(&p1); } } diff --git a/tests/isccfg/grammar_test.c b/tests/isccfg/grammar_test.c index ac06a7e49a..88e4f00160 100644 --- a/tests/isccfg/grammar_test.c +++ b/tests/isccfg/grammar_test.c @@ -124,16 +124,12 @@ static void test__query_source_print(const char *config, const char *expected) { isc_result_t result; isc_buffer_t buffer; - cfg_parser_t *parser = NULL; cfg_obj_t *output_conf = NULL; - result = cfg_parser_create(mctx, &parser); - assert_int_equal(result, ISC_R_SUCCESS); - isc_buffer_constinit(&buffer, config, strlen(config)); isc_buffer_add(&buffer, strlen(config)); - result = cfg_parse_buffer(parser, &buffer, "text1", 0, + result = cfg_parse_buffer(isc_g_mctx, &buffer, "text1", 0, &cfg_type_namedconf, 0, &output_conf); assert_int_equal(result, ISC_R_SUCCESS); assert_non_null(output_conf); @@ -144,9 +140,7 @@ test__query_source_print(const char *config, const char *expected) { output_conf->type->print(&pctx, output_conf); assert_text(expected); - cfg_obj_detach(parser, &output_conf); - cfg_parser_reset(parser); - cfg_parser_destroy(&parser); + cfg_obj_detach(&output_conf); } ISC_RUN_TEST_IMPL(query_source_print_none) { diff --git a/tests/isccfg/meson.build b/tests/isccfg/meson.build index 16ca023f86..419986f595 100644 --- a/tests/isccfg/meson.build +++ b/tests/isccfg/meson.build @@ -11,6 +11,7 @@ foreach unit : [ 'duration', + 'grammar', 'parser', ] test_bin = executable( diff --git a/tests/isccfg/parser_test.c b/tests/isccfg/parser_test.c index 8ace4ea4f4..a08bf2abe6 100644 --- a/tests/isccfg/parser_test.c +++ b/tests/isccfg/parser_test.c @@ -60,7 +60,6 @@ append(void *arg, const char *str, int len) { ISC_RUN_TEST_IMPL(addzoneconf) { isc_result_t result; isc_buffer_t b; - cfg_parser_t *p = NULL; const char *tests[] = { "zone \"test4.baz\" { type primary; file \"e.db\"; };", "zone \"test/.baz\" { type primary; file \"e.db\"; };", @@ -73,9 +72,6 @@ ISC_RUN_TEST_IMPL(addzoneconf) { char buf[1024]; /* Parse with default line numbering */ - result = cfg_parser_create(isc_g_mctx, &p); - assert_int_equal(result, ISC_R_SUCCESS); - for (size_t i = 0; i < ARRAY_SIZE(tests); i++) { cfg_obj_t *conf = NULL; const cfg_obj_t *obj = NULL, *zlist = NULL; @@ -83,7 +79,7 @@ ISC_RUN_TEST_IMPL(addzoneconf) { isc_buffer_constinit(&b, tests[i], strlen(tests[i])); isc_buffer_add(&b, strlen(tests[i])); - result = cfg_parse_buffer(p, &b, "text1", 0, + result = cfg_parse_buffer(isc_g_mctx, &b, "text1", 0, &cfg_type_namedconf, 0, &conf); assert_int_equal(result, ISC_R_SUCCESS); @@ -103,49 +99,77 @@ ISC_RUN_TEST_IMPL(addzoneconf) { assert_string_equal(tests[i], buf); cfg_obj_detach(&conf); - cfg_parser_reset(p); } - - cfg_parser_destroy(&p); } /* test cfg_parse_buffer() */ ISC_RUN_TEST_IMPL(parse_buffer) { isc_result_t result; - unsigned char text[] = "options\n{\nrecursion yes;\n};\n"; - isc_buffer_t buf1, buf2; - cfg_parser_t *p1 = NULL, *p2 = NULL; - cfg_obj_t *c1 = NULL, *c2 = NULL; + int fresult; + unsigned char text[] = "options\n{\nidonotexists yes;\n};\n"; + char logfilebuf[512]; + size_t logfilelen; + isc_buffer_t buf; + cfg_obj_t *c = NULL; - isc_buffer_init(&buf1, &text[0], sizeof(text) - 1); - isc_buffer_add(&buf1, sizeof(text) - 1); + /* + * Redirect parser errors into a specific file for checking the output + * later. + */ + constexpr char logfilename[] = "./cfglog.out"; + FILE *logfile = fopen(logfilename, "w+"); + assert_non_null(logfile); - /* Parse with default line numbering */ - result = cfg_parser_create(isc_g_mctx, &p1); - assert_int_equal(result, ISC_R_SUCCESS); + isc_logdestination_t *logdest = ISC_LOGDESTINATION_FILE(logfile); + isc_logconfig_t *logconfig = isc_logconfig_get(); + isc_log_createandusechannel(logconfig, "default_stderr", + ISC_LOG_TOFILEDESC, ISC_LOG_DYNAMIC, + logdest, 0, ISC_LOGCATEGORY_DEFAULT, + ISC_LOGMODULE_DEFAULT); - result = cfg_parse_buffer(p1, &buf1, "text1", 0, &cfg_type_namedconf, 0, - &c1); - assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(p1->line, 5); + /* Parse with default line numbering. */ + isc_buffer_init(&buf, &text[0], sizeof(text) - 1); + isc_buffer_add(&buf, sizeof(text) - 1); + result = cfg_parse_buffer(isc_g_mctx, &buf, "text1", 0, + &cfg_type_namedconf, 0, &c); + assert_int_equal(result, ISC_R_FAILURE); + assert_null(c); - isc_buffer_init(&buf2, &text[0], sizeof(text) - 1); - isc_buffer_add(&buf2, sizeof(text) - 1); + /* Parse with changed line number. */ + isc_buffer_first(&buf); + result = cfg_parse_buffer(isc_g_mctx, &buf, "text2", 100, + &cfg_type_namedconf, 0, &c); + assert_int_equal(result, ISC_R_FAILURE); + assert_null(c); - /* Parse with changed line number */ - result = cfg_parser_create(isc_g_mctx, &p2); - assert_int_equal(result, ISC_R_SUCCESS); + /* Parse with changed line number and no name. */ + isc_buffer_first(&buf); + result = cfg_parse_buffer(isc_g_mctx, &buf, NULL, 100, + &cfg_type_namedconf, 0, &c); + assert_int_equal(result, ISC_R_FAILURE); + assert_null(c); - result = cfg_parse_buffer(p2, &buf2, "text2", 100, &cfg_type_namedconf, - 0, &c2); - assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(p2->line, 104); + /* Check log values (and, specifically, line numbers). */ + logfilelen = ftell(logfile); + assert_in_range(logfilelen, 0, sizeof(logfilebuf)); - cfg_obj_detach(&c1); - cfg_obj_detach(&c2); + fresult = fseek(logfile, 0, SEEK_SET); + assert_int_equal(fresult, 0); - cfg_parser_destroy(&p1); - cfg_parser_destroy(&p2); + fresult = fread(logfilebuf, 1, logfilelen, logfile); + assert_int_equal(fresult, logfilelen); + + logfilebuf[logfilelen] = 0; + + assert_non_null( + strstr(logfilebuf, "text1:3: unknown option 'idonotexists'")); + assert_non_null( + strstr(logfilebuf, "text2:102: unknown option 'idonotexists'")); + assert_non_null( + strstr(logfilebuf, "none:102: unknown option 'idonotexists'")); + + fclose(logfile); + remove(logfilename); } /* test cfg_map_firstclause() */ From 4f7f2dae59a757dc7d1ce1840beebc0f3183db8a Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 22 Oct 2025 10:41:05 -0700 Subject: [PATCH 08/12] simplify cfg_parser API - the cfg_parser_create() and cfg_parser_destroy() calls are no longer used outside parser.c, so they are now static functions - cfg_parser_attach(), cfg_parser_reset(), and cfg_parser_setflags() are no longer used at all, and have been removed. - cfg_parser_mapadd() has been renamed for clarity to cfg_map_add(). --- bin/named/server.c | 4 +- lib/isccfg/include/isccfg/cfg.h | 64 +++++-------------------- lib/isccfg/parser.c | 83 ++++++++++----------------------- 3 files changed, 37 insertions(+), 114 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 989bcee49c..b5467a52b4 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -13236,7 +13236,7 @@ do_addzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, cfg_obj_attach(zoneconf, &cfg->nzf_config); } else { cfg_obj_t *z = UNCONST(zoneobj); - CHECK(cfg_parser_mapadd(cfg->nzf_config, z, "zone")); + CHECK(cfg_map_add(cfg->nzf_config, z, "zone")); } cleanup_config = true; #endif /* HAVE_LMDB */ @@ -13487,7 +13487,7 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, #ifndef HAVE_LMDB /* Store the new zone configuration; also in NZF if applicable */ z = UNCONST(zoneobj); - CHECK(cfg_parser_mapadd(cfg->nzf_config, z, "zone")); + CHECK(cfg_map_add(cfg->nzf_config, z, "zone")); #endif /* HAVE_LMDB */ if (added) { diff --git a/lib/isccfg/include/isccfg/cfg.h b/lib/isccfg/include/isccfg/cfg.h index b403fa7b28..462c112bda 100644 --- a/lib/isccfg/include/isccfg/cfg.h +++ b/lib/isccfg/include/isccfg/cfg.h @@ -85,34 +85,6 @@ typedef isc_result_t (*cfg_parsecallback_t)(const char *clausename, *** Functions ***/ -void -cfg_parser_attach(cfg_parser_t *src, cfg_parser_t **dest); -/*%< - * Reference a parser object. - */ - -isc_result_t -cfg_parser_create(isc_mem_t *mctx, cfg_parser_t **ret); -/*%< - * Create a configuration file parser. Any warning and error - * messages will be logged. - * - * The parser object returned can be used for a single call - * to cfg_parse_file() or cfg_parse_buffer(). It must not - * be reused for parsing multiple files or buffers. - */ - -void -cfg_parser_setflags(cfg_parser_t *pctx, unsigned int flags, bool turn_on); -/*%< - * Set parser context flags. The flags are not checked for sensibility. - * If 'turn_on' is 'true' the flags will be set, otherwise the flags will - * be cleared. - * - * Requires: - *\li "pctx" is not NULL. - */ - isc_result_t cfg_parse_file(isc_mem_t *mctx, const char *file, const cfg_type_t *type, unsigned int flags, cfg_obj_t **ret); @@ -152,31 +124,6 @@ cfg_parse_buffer(isc_mem_t *mctx, isc_buffer_t *buffer, const char *file, *\li others - file contains errors */ -isc_result_t -cfg_parser_mapadd(cfg_obj_t *mapobj, cfg_obj_t *obj, const char *clause); -/*%< - * Add the object 'obj' to the specified clause in mapbody 'mapobj'. - * Used for adding new zones. - * - * Require: - * \li 'obj' is a valid cfg_obj_t. - * \li 'mapobj' is a valid cfg_obj_t of type map. - */ - -void -cfg_parser_reset(cfg_parser_t *pctx); -/*%< - * Reset an existing parser so it can be re-used for a new file or - * buffer. - */ - -void -cfg_parser_destroy(cfg_parser_t **pctxp); -/*%< - * Remove a reference to a configuration parser; destroy it if there are no - * more references. - */ - cfg_obj_t * cfg_parser_currentfile(cfg_parser_t *pctx); /*%< @@ -225,6 +172,17 @@ cfg_map_get(const cfg_obj_t *mapobj, const char *name, const cfg_obj_t **obj); * \li #ISC_R_NOTFOUND - name not found in map */ +isc_result_t +cfg_map_add(cfg_obj_t *mapobj, cfg_obj_t *obj, const char *clause); +/*%< + * Add the object 'obj' to the specified clause in mapbody 'mapobj'. + * Used for adding new zones. + * + * Require: + * \li 'obj' is a valid cfg_obj_t. + * \li 'mapobj' is a valid cfg_obj_t of type map. + */ + const cfg_obj_t * cfg_map_getname(const cfg_obj_t *mapobj); /*%< diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index b2667a2629..072a1dc8ff 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -519,8 +519,8 @@ static cfg_type_t cfg_type_filelist = { "filelist", NULL, print_list, NULL, &cfg_rep_list, &cfg_type_qstring }; -isc_result_t -cfg_parser_create(isc_mem_t *mctx, cfg_parser_t **ret) { +static isc_result_t +parser_create(isc_mem_t *mctx, cfg_parser_t **ret) { isc_result_t result; cfg_parser_t *pctx; isc_lexspecials_t specials; @@ -582,14 +582,24 @@ cleanup: return result; } -void -cfg_parser_setflags(cfg_parser_t *pctx, unsigned int flags, bool turn_on) { - REQUIRE(pctx != NULL); +static void +parser_destroy(cfg_parser_t **pctxp) { + cfg_parser_t *pctx; - if (turn_on) { - pctx->flags |= flags; - } else { - pctx->flags &= ~flags; + REQUIRE(pctxp != NULL && *pctxp != NULL); + pctx = *pctxp; + *pctxp = NULL; + + if (isc_refcount_decrement(&pctx->references) == 1) { + isc_lex_destroy(&pctx->lexer); + /* + * Cleaning up open_files does not + * close the files; that was already done + * by closing the lexer. + */ + CLEANUP_OBJ(pctx->open_files); + CLEANUP_OBJ(pctx->closed_files); + isc_mem_putanddetach(&pctx->mctx, pctx, sizeof(*pctx)); } } @@ -617,21 +627,6 @@ cleanup: return result; } -void -cfg_parser_reset(cfg_parser_t *pctx) { - REQUIRE(pctx != NULL); - - if (pctx->lexer != NULL) { - isc_lex_close(pctx->lexer); - } - - pctx->seen_eof = false; - pctx->ungotten = false; - pctx->errors = 0; - pctx->warnings = 0; - pctx->line = 0; -} - /* * Parse a configuration using a pctx where a lexer has already * been set up with a source. @@ -685,7 +680,7 @@ cfg_parse_file(isc_mem_t *mctx, const char *filename, const cfg_type_t *type, REQUIRE(ret != NULL && *ret == NULL); REQUIRE_PCTX_FLAGS(flags); - CHECK(cfg_parser_create(mctx, &pctx)); + CHECK(parser_create(mctx, &pctx)); pctx->flags = flags; CHECK(parser_openfile(pctx, filename)); @@ -700,7 +695,7 @@ cfg_parse_file(isc_mem_t *mctx, const char *filename, const cfg_type_t *type, cleanup: if (pctx != NULL) { - cfg_parser_destroy(&pctx); + parser_destroy(&pctx); } return result; @@ -719,7 +714,7 @@ cfg_parse_buffer(isc_mem_t *mctx, isc_buffer_t *buffer, const char *file, REQUIRE(ret != NULL && *ret == NULL); REQUIRE_PCTX_FLAGS(flags); - CHECK(cfg_parser_create(mctx, &pctx)); + CHECK(parser_create(mctx, &pctx)); CHECK(isc_lex_openbuffer(pctx->lexer, buffer)); pctx->buf_name = file; @@ -734,42 +729,12 @@ cfg_parse_buffer(isc_mem_t *mctx, isc_buffer_t *buffer, const char *file, cleanup: if (pctx != NULL) { - cfg_parser_destroy(&pctx); + parser_destroy(&pctx); } return result; } -void -cfg_parser_attach(cfg_parser_t *src, cfg_parser_t **dest) { - REQUIRE(src != NULL); - REQUIRE(dest != NULL && *dest == NULL); - - isc_refcount_increment(&src->references); - *dest = src; -} - -void -cfg_parser_destroy(cfg_parser_t **pctxp) { - cfg_parser_t *pctx; - - REQUIRE(pctxp != NULL && *pctxp != NULL); - pctx = *pctxp; - *pctxp = NULL; - - if (isc_refcount_decrement(&pctx->references) == 1) { - isc_lex_destroy(&pctx->lexer); - /* - * Cleaning up open_files does not - * close the files; that was already done - * by closing the lexer. - */ - CLEANUP_OBJ(pctx->open_files); - CLEANUP_OBJ(pctx->closed_files); - isc_mem_putanddetach(&pctx->mctx, pctx, sizeof(*pctx)); - } -} - /* * void */ @@ -3927,7 +3892,7 @@ cfg_print_grammar(const cfg_type_t *type, unsigned int flags, } isc_result_t -cfg_parser_mapadd(cfg_obj_t *mapobj, cfg_obj_t *obj, const char *clausename) { +cfg_map_add(cfg_obj_t *mapobj, cfg_obj_t *obj, const char *clausename) { isc_result_t result = ISC_R_SUCCESS; const cfg_map_t *map = NULL; isc_symvalue_t symval; From 0db377da57b531279c327c558ee8451f91bbf93f Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 22 Oct 2025 14:51:15 -0700 Subject: [PATCH 09/12] simplify and regularize cfg_* functions - several functions that can no longer fail have been changed to type void, and unnecessary 'cleanup' sections were removed - renamed cfg_create_obj() to cfg_obj_create(), and cfg_create_tuple() to cfg_tuple_create(), to match typical nomenclature. - fixed a memory leak bug, in which an element could be removed from a list in delete_zoneconf() without being freed. this has been addressed by adding a cfg_list_unlink() function. list elements are now allocated based on the list they will be stored in, using the same mctx. --- bin/named/server.c | 6 +- lib/isccfg/include/isccfg/cfg.h | 7 + lib/isccfg/include/isccfg/grammar.h | 16 +- lib/isccfg/namedconf.c | 59 +++--- lib/isccfg/parser.c | 302 ++++++++++++---------------- 5 files changed, 173 insertions(+), 217 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index b5467a52b4..dbf5dc52bf 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -13089,7 +13089,6 @@ delete_zoneconf(dns_view_t *view, const cfg_obj_t *config, dns_name_t *myname = dns_fixedname_initname(&myfixed); const cfg_obj_t *zconf = cfg_listelt_value(elt); const char *zn = NULL; - cfg_listelt_t *e = NULL; zn = cfg_obj_asstring(cfg_tuple_get(zconf, "name")); result = dns_name_fromstring(myname, zn, dns_rootname, 0, NULL); @@ -13097,9 +13096,8 @@ delete_zoneconf(dns_view_t *view, const cfg_obj_t *config, continue; } - e = UNCONST(elt); - ISC_LIST_UNLINK(*list, e, link); - cfg_obj_detach(&e->obj); + cfg_obj_t *zones = UNCONST(zl); + cfg_list_unlink(zones, elt); result = ISC_R_SUCCESS; break; } diff --git a/lib/isccfg/include/isccfg/cfg.h b/lib/isccfg/include/isccfg/cfg.h index 462c112bda..90c94c1b73 100644 --- a/lib/isccfg/include/isccfg/cfg.h +++ b/lib/isccfg/include/isccfg/cfg.h @@ -441,6 +441,13 @@ cfg_list_length(const cfg_obj_t *obj, bool recurse); * all contained lists. */ +void +cfg_list_unlink(cfg_obj_t *list, cfg_listelt_t *elt); +/*%< + * Unlink 'elt' from the list object 'list', and free the memory associated + * with 'elt'. + */ + cfg_obj_t * cfg_listelt_value(const cfg_listelt_t *elt); /*%< diff --git a/lib/isccfg/include/isccfg/grammar.h b/lib/isccfg/include/isccfg/grammar.h index 8e84e31a37..b91c090130 100644 --- a/lib/isccfg/include/isccfg/grammar.h +++ b/lib/isccfg/include/isccfg/grammar.h @@ -349,8 +349,8 @@ cfg_ungettoken(cfg_parser_t *pctx); #define CFG_LEXOPT_QSTRING (ISC_LEXOPT_QSTRING | ISC_LEXOPT_QSTRINGMULTILINE) -isc_result_t -cfg_create_obj(isc_mem_t *mctx, cfg_obj_t *file, size_t line, +void +cfg_obj_create(isc_mem_t *mctx, cfg_obj_t *file, size_t line, const cfg_type_t *type, cfg_obj_t **ret); void @@ -419,8 +419,8 @@ isc_result_t cfg_parse_special(cfg_parser_t *pctx, int special); /*%< Parse a required special character 'special'. */ -isc_result_t -cfg_create_tuple(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **objp); +void +cfg_tuple_create(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **objp); isc_result_t cfg_parse_tuple(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret); @@ -432,12 +432,8 @@ void cfg_doc_tuple(cfg_printer_t *pctx, const cfg_type_t *type); isc_result_t -cfg_create_list(isc_mem_t *mctx, cfg_obj_t *file, size_t line, - const cfg_type_t *type, cfg_obj_t **objp); - -isc_result_t -cfg_parse_listelt(cfg_parser_t *pctx, const cfg_type_t *elttype, - cfg_listelt_t **ret); +cfg_parse_listelt(cfg_parser_t *pctx, cfg_obj_t *list, + const cfg_type_t *elttype, cfg_listelt_t **ret); isc_result_t cfg_parse_bracketed_list(cfg_parser_t *pctx, const cfg_type_t *type, diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c index d13fd830f8..5d0117ac5a 100644 --- a/lib/isccfg/namedconf.c +++ b/lib/isccfg/namedconf.c @@ -414,8 +414,8 @@ parse_updatepolicy(cfg_parser_t *pctx, const cfg_type_t *type, strcasecmp(TOKEN_STRING(pctx), "local") == 0) { cfg_obj_t *obj = NULL; - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_ustring, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_ustring, &obj); obj->value.string.length = strlen("local"); obj->value.string.base = isc_mem_get(pctx->mctx, obj->value.string.length + 1); @@ -973,8 +973,9 @@ parse_qstringornone(cfg_parser_t *pctx, const cfg_type_t *type, if (pctx->token.type == isc_tokentype_string && strcasecmp(TOKEN_STRING(pctx), "none") == 0) { - return cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_none, ret); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_none, ret); + return ISC_R_SUCCESS; } cfg_ungettoken(pctx); return cfg_parse_qstring(pctx, type, ret); @@ -1016,8 +1017,9 @@ parse_boolorauto(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { if (pctx->token.type == isc_tokentype_string && strcasecmp(TOKEN_STRING(pctx), "auto") == 0) { - return cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_auto, ret); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_auto, ret); + return ISC_R_SUCCESS; } cfg_ungettoken(pctx); return cfg_parse_boolean(pctx, type, ret); @@ -1071,19 +1073,17 @@ parse_serverid(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { if (pctx->token.type == isc_tokentype_string && strcasecmp(TOKEN_STRING(pctx), "none") == 0) { - return cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_none, ret); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_none, ret); + return ISC_R_SUCCESS; } if (pctx->token.type == isc_tokentype_string && strcasecmp(TOKEN_STRING(pctx), "hostname") == 0) { - result = cfg_create_obj(pctx->mctx, - cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_hostname, ret); - if (result == ISC_R_SUCCESS) { - (*ret)->value.boolean = true; - } - return result; + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_hostname, ret); + (*ret)->value.boolean = true; + return ISC_R_SUCCESS; } cfg_ungettoken(pctx); return cfg_parse_qstring(pctx, type, ret); @@ -1525,7 +1525,7 @@ parse_dtout(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { cfg_obj_t *obj = NULL; const cfg_tuplefielddef_t *fields = type->of; - CHECK(cfg_create_tuple(pctx, type, &obj)); + cfg_tuple_create(pctx, type, &obj); /* Parse the mandatory "mode" and "path" fields */ CHECK(cfg_parse_obj(pctx, fields[0].type, &obj->value.tuple[0])); @@ -1677,7 +1677,7 @@ cfg_parse_rpz_policy(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t *obj = NULL; const cfg_tuplefielddef_t *fields; - CHECK(cfg_create_tuple(pctx, type, &obj)); + cfg_tuple_create(pctx, type, &obj); fields = type->of; CHECK(cfg_parse_obj(pctx, fields[0].type, &obj->value.tuple[0])); @@ -1711,7 +1711,7 @@ cfg_parse_kv_tuple(cfg_parser_t *pctx, const cfg_type_t *type, int fn; isc_result_t result; - CHECK(cfg_create_tuple(pctx, type, &obj)); + cfg_tuple_create(pctx, type, &obj); /* * The zone first field is required and always first. @@ -2820,8 +2820,8 @@ parse_sizeval(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { } CHECK(parse_unitstring(TOKEN_STRING(pctx), &val)); - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_uint64, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + &cfg_type_uint64, &obj); obj->value.uint64 = val; *ret = obj; return ISC_R_SUCCESS; @@ -2852,15 +2852,15 @@ parse_sizeval_percent(cfg_parser_t *pctx, const cfg_type_t *type, percent = strtoull(TOKEN_STRING(pctx), &endp, 10); if (*endp == '%' && *(endp + 1) == 0) { - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_percentage, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_percentage, &obj); obj->value.uint32 = (uint32_t)percent; *ret = obj; return ISC_R_SUCCESS; } else { CHECK(parse_unitstring(TOKEN_STRING(pctx), &val)); - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_uint64, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_uint64, &obj); obj->value.uint64 = val; *ret = obj; return ISC_R_SUCCESS; @@ -3287,8 +3287,8 @@ parse_querysource(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { strcasecmp(TOKEN_STRING(pctx), "none") == 0) { CHECK(cfg_gettoken(pctx, 0)); - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_none, ret)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_none, ret); } else { CHECK(cfg_parse_sockaddr_generic(pctx, &cfg_type_querysource, type, ret)); @@ -3489,9 +3489,8 @@ parse_logseverity(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { * This makes little sense, but we support it for * compatibility with BIND 8. */ - CHECK(cfg_create_obj( - pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_uint32, ret)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_uint32, ret); (*ret)->value.uint32 = 1; } (*ret)->type = &cfg_type_debuglevel; /* XXX kludge */ @@ -3546,7 +3545,7 @@ parse_logfile(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { cfg_obj_t *obj = NULL; const cfg_tuplefielddef_t *fields = type->of; - CHECK(cfg_create_tuple(pctx, type, &obj)); + cfg_tuple_create(pctx, type, &obj); /* Parse the mandatory "file" field */ CHECK(cfg_parse_obj(pctx, fields[0].type, &obj->value.tuple[0])); diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index 072a1dc8ff..9582da2e0e 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -114,20 +114,22 @@ print_list(cfg_printer_t *pctx, const cfg_obj_t *obj); static void free_list(cfg_obj_t *obj); -static isc_result_t -create_listelt(isc_mem_t *mctx, cfg_listelt_t **eltp); +static void +create_list(isc_mem_t *mctx, cfg_obj_t *file, size_t line, + const cfg_type_t *type, cfg_obj_t **obj); +static void +create_listelt(cfg_obj_t *list, cfg_listelt_t **eltp); -static isc_result_t +static void create_string(cfg_parser_t *pctx, const char *contents, const cfg_type_t *type, cfg_obj_t **ret); - static void free_string(cfg_obj_t *obj); static void free_sockaddrtls(cfg_obj_t *obj); -static isc_result_t +static void create_map(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **objp); static void @@ -303,9 +305,8 @@ cfg_printx(const cfg_obj_t *obj, unsigned int flags, /* Tuples. */ -isc_result_t -cfg_create_tuple(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { - isc_result_t result; +void +cfg_tuple_create(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { const cfg_tuplefielddef_t *fields; const cfg_tuplefielddef_t *f; cfg_obj_t *obj = NULL; @@ -322,21 +323,14 @@ cfg_create_tuple(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { nfields++; } - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, type, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + type, &obj); obj->value.tuple = isc_mem_cget(pctx->mctx, nfields, sizeof(cfg_obj_t *)); for (f = fields, i = 0; f->name != NULL; f++, i++) { obj->value.tuple[i] = NULL; } *ret = obj; - return ISC_R_SUCCESS; - -cleanup: - if (obj != NULL) { - isc_mem_put(pctx->mctx, obj, sizeof(*obj)); - } - return result; } isc_result_t @@ -353,7 +347,7 @@ cfg_parse_tuple(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { fields = type->of; - CHECK(cfg_create_tuple(pctx, type, &obj)); + cfg_tuple_create(pctx, type, &obj); for (f = fields, i = 0; f->name != NULL; f++, i++) { CHECK(cfg_parse_obj(pctx, f->type, &obj->value.tuple[i])); } @@ -519,9 +513,8 @@ static cfg_type_t cfg_type_filelist = { "filelist", NULL, print_list, NULL, &cfg_rep_list, &cfg_type_qstring }; -static isc_result_t +static void parser_create(isc_mem_t *mctx, cfg_parser_t **ret) { - isc_result_t result; cfg_parser_t *pctx; isc_lexspecials_t specials; @@ -562,24 +555,12 @@ parser_create(isc_mem_t *mctx, cfg_parser_t **ret) { ISC_LEXCOMMENT_CPLUSPLUS | ISC_LEXCOMMENT_SHELL); - CHECK(cfg_create_list(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_filelist, - &pctx->open_files)); - CHECK(cfg_create_list(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_filelist, - &pctx->closed_files)); + create_list(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + &cfg_type_filelist, &pctx->open_files); + create_list(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + &cfg_type_filelist, &pctx->closed_files); *ret = pctx; - return ISC_R_SUCCESS; - -cleanup: - if (pctx->lexer != NULL) { - isc_lex_destroy(&pctx->lexer); - } - CLEANUP_OBJ(pctx->open_files); - CLEANUP_OBJ(pctx->closed_files); - isc_mem_putanddetach(&pctx->mctx, pctx, sizeof(*pctx)); - return result; } static void @@ -616,8 +597,8 @@ parser_openfile(cfg_parser_t *pctx, const char *filename) { goto cleanup; } - CHECK(create_string(pctx, filename, &cfg_type_qstring, &stringobj)); - CHECK(create_listelt(pctx->mctx, &elt)); + create_string(pctx, filename, &cfg_type_qstring, &stringobj); + create_listelt(pctx->open_files, &elt); elt->obj = stringobj; ISC_LIST_APPEND(pctx->open_files->value.list, elt, link); @@ -680,7 +661,7 @@ cfg_parse_file(isc_mem_t *mctx, const char *filename, const cfg_type_t *type, REQUIRE(ret != NULL && *ret == NULL); REQUIRE_PCTX_FLAGS(flags); - CHECK(parser_create(mctx, &pctx)); + parser_create(mctx, &pctx); pctx->flags = flags; CHECK(parser_openfile(pctx, filename)); @@ -714,7 +695,7 @@ cfg_parse_buffer(isc_mem_t *mctx, isc_buffer_t *buffer, const char *file, REQUIRE(ret != NULL && *ret == NULL); REQUIRE_PCTX_FLAGS(flags); - CHECK(parser_create(mctx, &pctx)); + parser_create(mctx, &pctx); CHECK(isc_lex_openbuffer(pctx->lexer, buffer)); pctx->buf_name = file; @@ -745,8 +726,9 @@ cfg_parse_void(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { UNUSED(type); - return cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_void, ret); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + &cfg_type_void, ret); + return ISC_R_SUCCESS; } void @@ -804,8 +786,8 @@ cfg_parse_percentage(cfg_parser_t *pctx, const cfg_type_t *type, return ISC_R_UNEXPECTEDTOKEN; } - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_percentage, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + &cfg_type_percentage, &obj); obj->value.uint32 = (uint32_t)percent; *ret = obj; @@ -878,8 +860,8 @@ cfg_parse_fixedpoint(cfg_parser_t *pctx, const cfg_type_t *type, return ISC_R_UNEXPECTEDTOKEN; } - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_fixedpoint, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + &cfg_type_fixedpoint, &obj); obj->value.uint32 = strtoul(p, NULL, 10) * 100; switch (n3) { @@ -945,8 +927,8 @@ cfg_parse_uint32(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { return ISC_R_UNEXPECTEDTOKEN; } - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_uint32, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + &cfg_type_uint32, &obj); obj->value.uint32 = pctx->token.value.as_ulong; *ret = obj; @@ -1167,8 +1149,8 @@ parse_duration(cfg_parser_t *pctx, cfg_obj_t **ret) { goto cleanup; } - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_duration, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + &cfg_type_duration, &obj); obj->value.duration = duration; *ret = obj; @@ -1223,8 +1205,8 @@ cfg_parse_duration_or_unlimited(cfg_parser_t *pctx, const cfg_type_t *type, duration.iso8601 = false; duration.unlimited = true; - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_duration, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_duration, &obj); obj->value.duration = duration; *ret = obj; return ISC_R_SUCCESS; @@ -1269,28 +1251,21 @@ cfg_type_t cfg_type_duration_or_unlimited = { "duration_or_unlimited", */ /* Create a string object from a null-terminated C string. */ -static isc_result_t +static void create_string(cfg_parser_t *pctx, const char *contents, const cfg_type_t *type, cfg_obj_t **ret) { - isc_result_t result; cfg_obj_t *obj = NULL; int len; - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, type, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + type, &obj); len = strlen(contents); obj->value.string.length = len; obj->value.string.base = isc_mem_get(pctx->mctx, len + 1); - if (obj->value.string.base == 0) { - isc_mem_put(pctx->mctx, obj, sizeof(*obj)); - return ISC_R_NOMEMORY; - } memmove(obj->value.string.base, contents, len); obj->value.string.base[len] = '\0'; *ret = obj; -cleanup: - return result; } isc_result_t @@ -1307,7 +1282,9 @@ cfg_parse_qstring(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { cfg_parser_error(pctx, CFG_LOG_NEAR, "expected quoted string"); return ISC_R_UNEXPECTEDTOKEN; } - return create_string(pctx, TOKEN_STRING(pctx), &cfg_type_qstring, ret); + create_string(pctx, TOKEN_STRING(pctx), &cfg_type_qstring, ret); + return ISC_R_SUCCESS; + cleanup: return result; } @@ -1324,7 +1301,9 @@ parse_ustring(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { "expected unquoted string"); return ISC_R_UNEXPECTEDTOKEN; } - return create_string(pctx, TOKEN_STRING(pctx), &cfg_type_ustring, ret); + create_string(pctx, TOKEN_STRING(pctx), &cfg_type_ustring, ret); + return ISC_R_SUCCESS; + cleanup: return result; } @@ -1339,7 +1318,9 @@ cfg_parse_astring(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { UNUSED(type); CHECK(cfg_getstringtoken(pctx)); - return create_string(pctx, TOKEN_STRING(pctx), &cfg_type_qstring, ret); + create_string(pctx, TOKEN_STRING(pctx), &cfg_type_qstring, ret); + return ISC_R_SUCCESS; + cleanup: return result; } @@ -1354,7 +1335,9 @@ cfg_parse_sstring(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { UNUSED(type); CHECK(cfg_getstringtoken(pctx)); - return create_string(pctx, TOKEN_STRING(pctx), &cfg_type_sstring, ret); + create_string(pctx, TOKEN_STRING(pctx), &cfg_type_sstring, ret); + return ISC_R_SUCCESS; + cleanup: return result; } @@ -1370,8 +1353,9 @@ parse_btext(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { cfg_parser_error(pctx, CFG_LOG_NEAR, "expected bracketed text"); return ISC_R_UNEXPECTEDTOKEN; } - return create_string(pctx, TOKEN_STRING(pctx), &cfg_type_bracketed_text, - ret); + create_string(pctx, TOKEN_STRING(pctx), &cfg_type_bracketed_text, ret); + return ISC_R_SUCCESS; + cleanup: return result; } @@ -1640,7 +1624,7 @@ parse_geoip(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { cfg_obj_t *obj = NULL; const cfg_tuplefielddef_t *fields = type->of; - CHECK(cfg_create_tuple(pctx, type, &obj)); + cfg_tuple_create(pctx, type, &obj); CHECK(cfg_parse_void(pctx, NULL, &obj->value.tuple[0])); /* Parse the optional "db" field. */ @@ -1887,18 +1871,15 @@ cfg_parse_boolean(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { goto bad_boolean; } - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_boolean, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + &cfg_type_boolean, &obj); obj->value.boolean = value; *ret = obj; - return result; + return ISC_R_SUCCESS; bad_boolean: cfg_parser_error(pctx, CFG_LOG_NEAR, "boolean expected"); return ISC_R_UNEXPECTEDTOKEN; - -cleanup: - return result; } void @@ -1921,73 +1902,64 @@ cfg_type_t cfg_type_boolean = { "boolean", cfg_parse_boolean, * Lists. */ -isc_result_t -cfg_create_list(isc_mem_t *mctx, cfg_obj_t *file, size_t line, - const cfg_type_t *type, cfg_obj_t **obj) { - isc_result_t result; - +static void +create_list(isc_mem_t *mctx, cfg_obj_t *file, size_t line, + const cfg_type_t *type, cfg_obj_t **obj) { REQUIRE(mctx != NULL); REQUIRE(type != NULL); REQUIRE(obj != NULL && *obj == NULL); - CHECK(cfg_create_obj(mctx, file, line, type, obj)); + cfg_obj_create(mctx, file, line, type, obj); ISC_LIST_INIT((*obj)->value.list); -cleanup: - return result; -} - -static isc_result_t -create_listelt(isc_mem_t *mctx, cfg_listelt_t **eltp) { - cfg_listelt_t *elt; - - elt = isc_mem_get(mctx, sizeof(*elt)); - elt->obj = NULL; - ISC_LINK_INIT(elt, link); - *eltp = elt; - return ISC_R_SUCCESS; } static void -free_listelt(isc_mem_t *mctx, cfg_listelt_t *elt) { +create_listelt(cfg_obj_t *list, cfg_listelt_t **eltp) { + cfg_listelt_t *elt = isc_mem_get(list->mctx, sizeof(*elt)); + *elt = (cfg_listelt_t){ .link = ISC_LINK_INITIALIZER }; + *eltp = elt; +} + +static void +free_listelt(cfg_obj_t *list, cfg_listelt_t **eltp) { + cfg_listelt_t *elt = *eltp; + *eltp = NULL; + if (elt->obj != NULL) { cfg_obj_detach(&elt->obj); } - isc_mem_put(mctx, elt, sizeof(*elt)); + isc_mem_put(list->mctx, elt, sizeof(*elt)); } static void free_list(cfg_obj_t *obj) { ISC_LIST_FOREACH(obj->value.list, elt, link) { - free_listelt(obj->mctx, elt); + free_listelt(obj, &elt); } } isc_result_t -cfg_parse_listelt(cfg_parser_t *pctx, const cfg_type_t *elttype, - cfg_listelt_t **ret) { +cfg_parse_listelt(cfg_parser_t *pctx, cfg_obj_t *list, + const cfg_type_t *elttype, cfg_listelt_t **ret) { isc_result_t result; cfg_listelt_t *elt = NULL; cfg_obj_t *value = NULL; REQUIRE(pctx != NULL); + REQUIRE(list != NULL); REQUIRE(elttype != NULL); REQUIRE(ret != NULL && *ret == NULL); - CHECK(create_listelt(pctx->mctx, &elt)); - result = cfg_parse_obj(pctx, elttype, &value); if (result != ISC_R_SUCCESS) { - goto cleanup; + return result; } + create_listelt(list, &elt); elt->obj = value; - *ret = elt; - return ISC_R_SUCCESS; -cleanup: - isc_mem_put(pctx->mctx, elt, sizeof(*elt)); - return result; + return ISC_R_SUCCESS; } /* @@ -2001,8 +1973,8 @@ parse_list(cfg_parser_t *pctx, const cfg_type_t *listtype, cfg_obj_t **ret) { isc_result_t result; cfg_listelt_t *elt = NULL; - CHECK(cfg_create_list(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, listtype, &listobj)); + create_list(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + listtype, &listobj); for (;;) { CHECK(cfg_peektoken(pctx, 0)); @@ -2011,7 +1983,7 @@ parse_list(cfg_parser_t *pctx, const cfg_type_t *listtype, cfg_obj_t **ret) { { break; } - CHECK(cfg_parse_listelt(pctx, listof, &elt)); + CHECK(cfg_parse_listelt(pctx, listobj, listof, &elt)); CHECK(parse_semicolon(pctx)); ISC_LIST_APPEND(listobj->value.list, elt, link); elt = NULL; @@ -2021,7 +1993,7 @@ parse_list(cfg_parser_t *pctx, const cfg_type_t *listtype, cfg_obj_t **ret) { cleanup: if (elt != NULL) { - free_listelt(pctx->mctx, elt); + free_listelt(listobj, &elt); } CLEANUP_OBJ(listobj); return result; @@ -2097,8 +2069,8 @@ cfg_parse_spacelist(cfg_parser_t *pctx, const cfg_type_t *listtype, listof = listtype->of; - CHECK(cfg_create_list(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, listtype, &listobj)); + create_list(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + listtype, &listobj); for (;;) { cfg_listelt_t *elt = NULL; @@ -2109,7 +2081,7 @@ cfg_parse_spacelist(cfg_parser_t *pctx, const cfg_type_t *listtype, { break; } - CHECK(cfg_parse_listelt(pctx, listof, &elt)); + CHECK(cfg_parse_listelt(pctx, listobj, listof, &elt)); ISC_LIST_APPEND(listobj->value.list, elt, link); } *ret = listobj; @@ -2158,6 +2130,12 @@ cfg_list_next(const cfg_listelt_t *elt) { return ISC_LIST_NEXT(elt, link); } +void +cfg_list_unlink(cfg_obj_t *list, cfg_listelt_t *elt) { + ISC_LIST_UNLINK(list->value.list, elt, link); + free_listelt(list, &elt); +} + /* * Return the length of a list object. If obj is NULL or is not * a list, return 0. @@ -2218,14 +2196,11 @@ cfg_parse_mapbody(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { clausesets = type->of; - CHECK(create_map(pctx, type, &obj)); + create_map(pctx, type, &obj); obj->value.map.clausesets = clausesets; for (;;) { - cfg_listelt_t *elt; - - redo: /* * Parse the option name and see if it is known. */ @@ -2287,7 +2262,7 @@ cfg_parse_mapbody(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { cfg_obj_detach(&includename); globfree(&g); - goto redo; + continue; } clause = NULL; @@ -2364,26 +2339,25 @@ cfg_parse_mapbody(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { if ((clause->flags & CFG_CLAUSEFLAG_MULTI) != 0) { /* Multivalued clause */ cfg_obj_t *listobj = NULL; + cfg_listelt_t *elt = NULL; - CHECK(cfg_create_list( - pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_implicitlist, &listobj)); + create_list(pctx->mctx, cfg_parser_currentfile(pctx), + pctx->line, &cfg_type_implicitlist, + &listobj); symval.as_pointer = listobj; result = isc_symtab_define_and_return( obj->value.map.symtab, clause->name, SYMTAB_DUMMY_TYPE, symval, isc_symexists_reject, &symval); - if (result == ISC_R_EXISTS) { CLEANUP_OBJ(listobj); listobj = symval.as_pointer; } - elt = NULL; - CHECK(cfg_parse_listelt(pctx, clause->type, &elt)); - CHECK(parse_semicolon(pctx)); - + CHECK(cfg_parse_listelt(pctx, listobj, clause->type, + &elt)); ISC_LIST_APPEND(listobj->value.list, elt, link); + CHECK(parse_semicolon(pctx)); } else { /* Single-valued clause */ bool chdir = ((clause->flags & CFG_CLAUSEFLAG_CHDIR) != @@ -2840,8 +2814,8 @@ parse_token(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { UNUSED(type); - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_token, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + &cfg_type_token, &obj); CHECK(cfg_gettoken(pctx, CFG_LEXOPT_QSTRING)); if (pctx->token.type == isc_tokentype_eof) { cfg_ungettoken(pctx); @@ -2880,8 +2854,8 @@ parse_unsupported(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { isc_result_t result; int braces = 0; - CHECK(cfg_create_list(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, type, &listobj)); + create_list(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, type, + &listobj); for (;;) { cfg_listelt_t *elt = NULL; @@ -2905,7 +2879,7 @@ parse_unsupported(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { goto cleanup; } - CHECK(cfg_parse_listelt(pctx, &cfg_type_token, &elt)); + CHECK(cfg_parse_listelt(pctx, listobj, &cfg_type_token, &elt)); ISC_LIST_APPEND(listobj->value.list, elt, link); } INSIST(braces == 0); @@ -3112,8 +3086,8 @@ parse_netaddr(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { isc_netaddr_t netaddr; unsigned int flags = *(const unsigned int *)type->of; - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, type, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + type, &obj); CHECK(cfg_parse_rawaddr(pctx, flags, &netaddr)); isc_sockaddr_fromnetaddr(&obj->value.sockaddr, &netaddr, 0); *ret = obj; @@ -3242,8 +3216,8 @@ cfg_parse_netprefix(cfg_parser_t *pctx, const cfg_type_t *type, } prefixlen = addrlen; } - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, &cfg_type_netprefix, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + &cfg_type_netprefix, &obj); obj->value.netprefix.address = netaddr; obj->value.netprefix.prefixlen = prefixlen; *ret = obj; @@ -3371,8 +3345,8 @@ parse_sockaddrsub(cfg_parser_t *pctx, const cfg_type_t *type, int flags, goto cleanup; } - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, type, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + type, &obj); if (have_tls == 1) { obj->value.sockaddrtls.tls = tls; } @@ -3761,8 +3735,8 @@ cfg_obj_line(const cfg_obj_t *obj) { return obj->line; } -isc_result_t -cfg_create_obj(isc_mem_t *mctx, cfg_obj_t *file, size_t line, +void +cfg_obj_create(isc_mem_t *mctx, cfg_obj_t *file, size_t line, const cfg_type_t *type, cfg_obj_t **ret) { cfg_obj_t *obj; @@ -3780,8 +3754,6 @@ cfg_create_obj(isc_mem_t *mctx, cfg_obj_t *file, size_t line, } *ret = obj; - - return ISC_R_SUCCESS; } static void @@ -3796,27 +3768,19 @@ map_symtabitem_destroy(char *key, unsigned int type, isc_symvalue_t symval, cfg_obj_detach(&obj); } -static isc_result_t +static void create_map(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { - isc_result_t result; isc_symtab_t *symtab = NULL; cfg_obj_t *obj = NULL; - CHECK(cfg_create_obj(pctx->mctx, cfg_parser_currentfile(pctx), - pctx->line, type, &obj)); + cfg_obj_create(pctx->mctx, cfg_parser_currentfile(pctx), pctx->line, + type, &obj); isc_symtab_create(pctx->mctx, map_symtabitem_destroy, pctx, false, &symtab); obj->value.map.symtab = symtab; obj->value.map.id = NULL; *ret = obj; - return ISC_R_SUCCESS; - -cleanup: - if (obj != NULL) { - isc_mem_put(pctx->mctx, obj, sizeof(*obj)); - } - return result; } static void @@ -3834,7 +3798,7 @@ cfg_obj_istype(const cfg_obj_t *obj, const cfg_type_t *type) { } /* - * Destroy 'obj', a configuration object created in 'pctx'. + * Destroy 'obj'. */ static void cfg__obj_destroy(cfg_obj_t *obj) { @@ -3896,8 +3860,6 @@ cfg_map_add(cfg_obj_t *mapobj, cfg_obj_t *obj, const char *clausename) { isc_result_t result = ISC_R_SUCCESS; const cfg_map_t *map = NULL; isc_symvalue_t symval; - cfg_obj_t *destobj = NULL; - cfg_listelt_t *elt = NULL; const cfg_clausedef_t *const *clauseset = NULL; const cfg_clausedef_t *clause = NULL; @@ -3925,10 +3887,12 @@ breakout: &symval); if (result == ISC_R_NOTFOUND) { if ((clause->flags & CFG_CLAUSEFLAG_MULTI) != 0) { - CHECK(cfg_create_list(mapobj->mctx, obj->file, - obj->line, &cfg_type_implicitlist, - &destobj)); - CHECK(create_listelt(mapobj->mctx, &elt)); + cfg_obj_t *destobj = NULL; + cfg_listelt_t *elt = NULL; + + create_list(mapobj->mctx, obj->file, obj->line, + &cfg_type_implicitlist, &destobj); + create_listelt(destobj, &elt); cfg_obj_attach(obj, &elt->obj); ISC_LIST_APPEND(destobj->value.list, elt, link); symval.as_pointer = destobj; @@ -3941,28 +3905,20 @@ breakout: isc_symexists_reject); INSIST(result == ISC_R_SUCCESS); } else { - cfg_obj_t *destobj2 = symval.as_pointer; + cfg_obj_t *destobj = symval.as_pointer; + cfg_listelt_t *elt = NULL; INSIST(result == ISC_R_SUCCESS); - if (destobj2->type == &cfg_type_implicitlist) { - CHECK(create_listelt(mapobj->mctx, &elt)); + if (destobj->type == &cfg_type_implicitlist) { + create_listelt(destobj, &elt); cfg_obj_attach(obj, &elt->obj); - ISC_LIST_APPEND(destobj2->value.list, elt, link); + ISC_LIST_APPEND(destobj->value.list, elt, link); } else { result = ISC_R_EXISTS; } } - destobj = NULL; - elt = NULL; - -cleanup: - if (elt != NULL) { - free_listelt(mapobj->mctx, elt); - } - CLEANUP_OBJ(destobj); - return result; } From 6b5246b3d252773f0e14dd1264eca72ebc9a4936 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Thu, 23 Oct 2025 10:12:38 +0200 Subject: [PATCH 10/12] ensure parser/cfg_obj log includes the line number Since the `file` property of cfg_obj_t can now be null (instead of "none"), cfg_obj_t would take a fallback flow where the line was not logged. This fixes it. Also, add the log line when parser_complain is called and `file` is null (which might happend when parsing buffer only) to also include the line number. --- bin/tests/system/addzone/tests.sh | 8 ++++---- lib/isccfg/parser.c | 8 +++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/bin/tests/system/addzone/tests.sh b/bin/tests/system/addzone/tests.sh index 66c76bca18..ed897b9ba7 100755 --- a/bin/tests/system/addzone/tests.sh +++ b/bin/tests/system/addzone/tests.sh @@ -69,19 +69,19 @@ if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) nextpart ns2/named.run >/dev/null -echo_i "checking addzone errors are logged correctly" +echo_i "checking addzone errors are logged correctly ($n)" ret=0 $RNDCCMD 10.53.0.2 addzone bad.example '{ type mister; };' 2>&1 | grep 'unexpected token' >/dev/null 2>&1 || ret=1 -wait_for_log_peek 20 "addzone: 'mister' unexpected" ns2/named.run || ret=1 +wait_for_log_peek 20 "addzone:1: 'mister' unexpected" ns2/named.run || ret=1 n=$((n + 1)) if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) nextpart ns2/named.run >/dev/null -echo_i "checking modzone errors are logged correctly" +echo_i "checking modzone errors are logged correctly ($n)" ret=0 $RNDCCMD 10.53.0.2 modzone added.example '{ type mister; };' 2>&1 | grep 'unexpected token' >/dev/null 2>&1 || ret=1 -wait_for_log_peek 20 "modzone: 'mister' unexpected" ns2/named.run || ret=1 +wait_for_log_peek 20 "modzone:1: 'mister' unexpected" ns2/named.run || ret=1 n=$((n + 1)) if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index 9582da2e0e..f0af06a8bb 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -3648,8 +3648,10 @@ parser_complain(cfg_parser_t *pctx, bool is_warning, unsigned int flags, if (file != NULL) { snprintf(where, sizeof(where), "%s:%u: ", cfg_obj_asstring(file), pctx->line); - } else if (pctx->buf_name != NULL) { - snprintf(where, sizeof(where), "%s: ", pctx->buf_name); + } else { + snprintf(where, sizeof(where), "%s:%u: ", + pctx->buf_name == NULL ? "none" : pctx->buf_name, + pctx->line); } len = vsnprintf(message, sizeof(message), format, args); @@ -3717,7 +3719,7 @@ cfg_obj_log(const cfg_obj_t *obj, int level, const char *fmt, ...) { isc_log_write(CAT, MOD, level, "%s:%u: %s", cfg_obj_asstring(obj->file), obj->line, msgbuf); } else { - isc_log_write(CAT, MOD, level, "%s", msgbuf); + isc_log_write(CAT, MOD, level, "%u: %s", obj->line, msgbuf); } } From 6f4d4ddb1c9d00cdb0dd4f41f49b9db6be1b985c Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Thu, 23 Oct 2025 10:54:32 +0200 Subject: [PATCH 11/12] test rndc showzone works for named.conf zones Since the zone now has a reference to their respective configuration tree, `rndc showzone` can be used for any zones (including those defined in namedconf), without `allow-new-zones` being enabled. Add a test for this. The test is part of the addzone suite because showzone used to be related to addzone, but this could be moved elsewhere in the future if more specific tests are needed for showzone. --- .../ns1/{named.conf.in => named.conf.j2} | 5 +++ bin/tests/system/addzone/setup.sh | 1 - .../system/addzone/tests_showzone_static.py | 31 +++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) rename bin/tests/system/addzone/ns1/{named.conf.in => named.conf.j2} (88%) create mode 100644 bin/tests/system/addzone/tests_showzone_static.py diff --git a/bin/tests/system/addzone/ns1/named.conf.in b/bin/tests/system/addzone/ns1/named.conf.j2 similarity index 88% rename from bin/tests/system/addzone/ns1/named.conf.in rename to bin/tests/system/addzone/ns1/named.conf.j2 index 30731c46a6..c3302760ae 100644 --- a/bin/tests/system/addzone/ns1/named.conf.in +++ b/bin/tests/system/addzone/ns1/named.conf.j2 @@ -10,6 +10,7 @@ * See the COPYRIGHT file distributed with this work for additional * information regarding copyright ownership. */ +{% set allownewzones = allownewzones | default(True) %} key rndc_key { secret "1234abcd8765"; @@ -27,7 +28,11 @@ options { listen-on-v6 { none; }; allow-transfer { any; }; allow-query { any; }; +{% if allownewzones %} allow-new-zones yes; +{% else %} + allow-new-zones no; +{% endif %} recursion no; dnssec-validation no; }; diff --git a/bin/tests/system/addzone/setup.sh b/bin/tests/system/addzone/setup.sh index b2227c16b2..b6e778f629 100644 --- a/bin/tests/system/addzone/setup.sh +++ b/bin/tests/system/addzone/setup.sh @@ -17,7 +17,6 @@ cp -f ns1/redirect.db.1 ns1/redirect.db cp -f ns2/redirect.db.1 ns2/redirect.db cp -f ns3/redirect.db.1 ns3/redirect.db -copy_setports ns1/named.conf.in ns1/named.conf copy_setports ns2/named1.conf.in ns2/named.conf copy_setports ns3/named1.conf.in ns3/named.conf diff --git a/bin/tests/system/addzone/tests_showzone_static.py b/bin/tests/system/addzone/tests_showzone_static.py new file mode 100644 index 0000000000..6edaa497f6 --- /dev/null +++ b/bin/tests/system/addzone/tests_showzone_static.py @@ -0,0 +1,31 @@ +# Copyright (C) Internet Systems Consortium, Inc. ("ISC") +# +# SPDX-License-Identifier: MPL-2.0 +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, you can obtain one at https://mozilla.org/MPL/2.0/. +# +# See the COPYRIGHT file distributed with this work for additional +# information regarding copyright ownership. + +import pytest + + +# Test that `rndc showzone` can print any zone, including those statically +# defined in named.conf, and not only those added dynamically. +@pytest.mark.parametrize( + "allow", + [ + pytest.param(True, id="allow-new-zones-yes"), + pytest.param(False, id="allow-new-zones-no"), + ], +) +def test_showzone_static(ns1, templates, allow): + templates.render("ns1/named.conf", {"allownewzones": allow}) + ns1.rndc("reload", log=False) + zoneconfig = ns1.rndc("showzone inlinesec.example", log=False) + assert ( + zoneconfig + == 'zone "inlinesec.example" { type primary; file "inlinesec.db"; };\n' + ) From 2877b577356c9c6b25758f92b253157ddea32a48 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 23 Oct 2025 11:44:15 -0700 Subject: [PATCH 12/12] restore the former change_directory logging change_directory() now lives in libisccfg. when it was moved, the logging behavior changed: previously it had been logged by named only, in the general logging category, and without the named.conf filename and line number. it was not logged by named-checkconf. this behavior has now been restored. --- bin/named/server.c | 3 +-- lib/isccfg/parser.c | 40 ++++++++++++---------------------------- 2 files changed, 13 insertions(+), 30 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index dbf5dc52bf..5d7f74cb19 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -8143,8 +8143,7 @@ apply_configuration(cfg_obj_t *config, cfg_obj_t *bindkeys, if (getcwd(cwd, sizeof(cwd)) == cwd) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, - "the initial working directory is '%s'", - cwd); + "the working directory is now '%s'", cwd); } } diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index f0af06a8bb..f5e2dbffd9 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -136,8 +136,8 @@ static void free_map(cfg_obj_t *obj); static isc_result_t -parse_symtab_elt(cfg_parser_t *pctx, const char *name, cfg_type_t *elttype, - isc_symtab_t *symtab, bool callback); +parse_symtab_elt(cfg_parser_t *pctx, const cfg_clausedef_t *clause, + isc_symtab_t *symtab); static void free_noop(cfg_obj_t *obj); @@ -2360,12 +2360,8 @@ cfg_parse_mapbody(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { CHECK(parse_semicolon(pctx)); } else { /* Single-valued clause */ - bool chdir = ((clause->flags & CFG_CLAUSEFLAG_CHDIR) != - 0); - - result = parse_symtab_elt(pctx, clause->name, - clause->type, - obj->value.map.symtab, chdir); + result = parse_symtab_elt(pctx, clause, + obj->value.map.symtab); if (result == ISC_R_EXISTS) { cfg_parser_error(pctx, CFG_LOG_NEAR, "'%s' redefined", @@ -2392,19 +2388,13 @@ cleanup: } static isc_result_t -change_directory(const char *clausename, const cfg_obj_t *obj) { +change_directory(const cfg_obj_t *obj) { isc_result_t result; - const char *directory; - - REQUIRE(strcasecmp("directory", clausename) == 0); - - UNUSED(clausename); + const char *directory = cfg_obj_asstring(obj); /* * Change directory. */ - directory = cfg_obj_asstring(obj); - if (!isc_file_ischdiridempotent(directory)) { cfg_obj_log(obj, ISC_LOG_WARNING, "option 'directory' contains relative path '%s'", @@ -2425,30 +2415,24 @@ change_directory(const char *clausename, const cfg_obj_t *obj) { return result; } - char cwd[PATH_MAX]; - if (getcwd(cwd, sizeof(cwd)) == cwd) { - cfg_obj_log(obj, ISC_LOG_INFO, - "the working directory is now '%s'", cwd); - } - return ISC_R_SUCCESS; } static isc_result_t -parse_symtab_elt(cfg_parser_t *pctx, const char *name, cfg_type_t *elttype, - isc_symtab_t *symtab, bool chdir) { +parse_symtab_elt(cfg_parser_t *pctx, const cfg_clausedef_t *clause, + isc_symtab_t *symtab) { isc_result_t result; cfg_obj_t *obj = NULL; isc_symvalue_t symval; - CHECK(cfg_parse_obj(pctx, elttype, &obj)); + CHECK(cfg_parse_obj(pctx, clause->type, &obj)); - if (chdir) { - CHECK(change_directory(name, obj)); + if ((clause->flags & CFG_CLAUSEFLAG_CHDIR) != 0) { + CHECK(change_directory(obj)); } symval.as_pointer = obj; - CHECK(isc_symtab_define(symtab, name, SYMTAB_DUMMY_TYPE, symval, + CHECK(isc_symtab_define(symtab, clause->name, SYMTAB_DUMMY_TYPE, symval, isc_symexists_reject)); return ISC_R_SUCCESS;