Replace keystore attach/detach with ISC_REFCOUNT_IMPL/ISC_REFCOUNT_DECL

This is now the default way to implement attaching to/detaching from
a pointer.

Also update cfg_keystore_fromconfig() to allow NULL value for the
keystore pointer. In most cases we detach it immediately after the
function call.
This commit is contained in:
Matthijs Mekking 2024-01-03 16:24:53 +01:00
parent 2615b8a8b5
commit 8602beecd1
7 changed files with 36 additions and 87 deletions

View file

@ -276,14 +276,12 @@ kasp_from_conf(cfg_obj_t *config, isc_mem_t *mctx, const char *name,
cfg_obj_t *kconfig = cfg_listelt_value(element);
ks = NULL;
result = cfg_keystore_fromconfig(kconfig, mctx, lctx, engine,
&kslist, &ks);
&kslist, NULL);
if (result != ISC_R_SUCCESS) {
fatal("failed to configure key-store '%s': %s",
cfg_obj_asstring(cfg_tuple_get(kconfig, "name")),
isc_result_totext(result));
}
INSIST(ks != NULL);
dns_keystore_detach(&ks);
}
/* Default key-directory key store. */
ks = NULL;

View file

@ -8895,15 +8895,11 @@ load_configuration(const char *filename, named_server_t *server,
/*
* Create the built-in key store ("key-directory").
*/
keystore = NULL;
result = cfg_keystore_fromconfig(NULL, named_g_mctx, named_g_lctx,
named_g_engine, &keystorelist,
&keystore);
named_g_engine, &keystorelist, NULL);
if (result != ISC_R_SUCCESS) {
goto cleanup_keystorelist;
}
INSIST(keystore != NULL);
dns_keystore_detach(&keystore);
/*
* Create the DNSSEC key stores.
@ -8917,12 +8913,10 @@ load_configuration(const char *filename, named_server_t *server,
keystore = NULL;
result = cfg_keystore_fromconfig(kconfig, named_g_mctx,
named_g_lctx, named_g_engine,
&keystorelist, &keystore);
&keystorelist, NULL);
if (result != ISC_R_SUCCESS) {
goto cleanup_keystorelist;
}
INSIST(keystore != NULL);
dns_keystore_detach(&keystore);
}
/*

View file

@ -24,6 +24,8 @@
* A key store defines where to store DNSSEC keys.
*/
/* Add -DDNS_KEYSTORE_TRACE=1 to CFLAGS for detailed reference tracing */
#include <isc/lang.h>
#include <isc/magic.h>
#include <isc/mutex.h>
@ -85,38 +87,6 @@ dns_keystore_create(isc_mem_t *mctx, const char *name, const char *engine,
*\li Other errors are possible.
*/
void
dns_keystore_attach(dns_keystore_t *source, dns_keystore_t **targetp);
/*%<
* Attach '*targetp' to 'source'.
*
* Requires:
*
*\li 'source' is a valid keystore.
*
*\li 'targetp' points to a NULL dns_keystore_t *.
*
* Ensures:
*
*\li *targetp is attached to source.
*
*\li While *targetp is attached, the keystore will not shut down.
*/
void
dns_keystore_detach(dns_keystore_t **kspp);
/*%<
* Detach keystore.
*
* Requires:
*
*\li 'kspp' points to a valid dns_keystore_t *
*
* Ensures:
*
*\li *kspp is NULL.
*/
const char *
dns_keystore_name(dns_keystore_t *keystore);
/*%<
@ -231,4 +201,20 @@ dns_keystorelist_find(dns_keystorelist_t *list, const char *name,
*\li #ISC_R_NOTFOUND No matching keystore was found.
*/
#ifdef DNS_KEYSTORE_TRACE
/* Compatibility macros */
#define dns_keystore_attach(ks, ksp) \
dns_keystore__attach(ks, ksp, __func__, __FILE__, __LINE__)
#define dns_keystore_detach(ksp) \
dns_keystore__detach(ksp, __func__, __FILE__, __LINE__)
#define dns_keystore_ref(ptr) \
dns_keystore__ref(ptr, __func__, __FILE__, __LINE__)
#define dns_keystore_unref(ptr) \
dns_keystore__unref(ptr, __func__, __FILE__, __LINE__)
ISC_REFCOUNT_TRACE_DECL(dns_keystore);
#else
ISC_REFCOUNT_DECL(dns_keystore);
#endif /* DNS_KEYSTORE_TRACE */
ISC_LANG_ENDDECLS

View file

@ -54,17 +54,8 @@ dns_keystore_create(isc_mem_t *mctx, const char *name, const char *engine,
return (ISC_R_SUCCESS);
}
void
dns_keystore_attach(dns_keystore_t *source, dns_keystore_t **targetp) {
REQUIRE(DNS_KEYSTORE_VALID(source));
REQUIRE(targetp != NULL && *targetp == NULL);
isc_refcount_increment(&source->references);
*targetp = source;
}
static inline void
destroy(dns_keystore_t *keystore) {
dns__keystore_destroy(dns_keystore_t *keystore) {
char *name;
REQUIRE(!ISC_LINK_LINKED(keystore, link));
@ -81,17 +72,11 @@ destroy(dns_keystore_t *keystore) {
isc_mem_putanddetach(&keystore->mctx, keystore, sizeof(*keystore));
}
void
dns_keystore_detach(dns_keystore_t **kspp) {
REQUIRE(kspp != NULL && DNS_KEYSTORE_VALID(*kspp));
dns_keystore_t *ks = *kspp;
*kspp = NULL;
if (isc_refcount_decrement(&ks->references) == 1) {
destroy(ks);
}
}
#ifdef DNS_KEYSTORE_TRACE
ISC_REFCOUNT_TRACE_IMPL(dns_keystore, dns__keystore_destroy);
#else
ISC_REFCOUNT_IMPL(dns_keystore, dns__keystore_destroy);
#endif
const char *
dns_keystore_name(dns_keystore_t *keystore) {

View file

@ -1414,15 +1414,12 @@ check_options(const cfg_obj_t *options, const cfg_obj_t *config,
ret = cfg_keystore_fromconfig(kconfig, mctx,
logctx, NULL,
&kslist, &ks);
&kslist, NULL);
if (ret != ISC_R_SUCCESS) {
if (result == ISC_R_SUCCESS) {
result = ret;
}
}
if (ks != NULL) {
dns_keystore_detach(&ks);
}
}
}
}
@ -1431,15 +1428,12 @@ check_options(const cfg_obj_t *options, const cfg_obj_t *config,
* Add default key-store "key-directory".
*/
tresult = cfg_keystore_fromconfig(NULL, mctx, logctx, NULL, &kslist,
&ks);
NULL);
if (tresult != ISC_R_SUCCESS) {
if (result == ISC_R_SUCCESS) {
result = tresult;
}
}
if (ks != NULL) {
dns_keystore_detach(&ks);
}
/*
* Check dnssec-policy.
@ -2962,16 +2956,10 @@ check_keydir(const cfg_obj_t *config, const cfg_obj_t *zconfig,
element = cfg_list_next(element))
{
cfg_obj_t *kcfg = cfg_listelt_value(element);
ks = NULL;
(void)cfg_keystore_fromconfig(kcfg, mctx, logctx, NULL, &kslist,
&ks);
INSIST(ks != NULL);
dns_keystore_detach(&ks);
NULL);
}
ks = NULL;
(void)cfg_keystore_fromconfig(NULL, mctx, logctx, NULL, &kslist, &ks);
INSIST(ks != NULL);
dns_keystore_detach(&ks);
(void)cfg_keystore_fromconfig(NULL, mctx, logctx, NULL, &kslist, NULL);
/*
* Look for the dnssec-policy by name, which is the dnssec-policy
@ -3037,9 +3025,6 @@ check:
if (kasp != NULL) {
dns_kasp_detach(&kasp);
}
if (ks != NULL) {
dns_keystore_detach(&ks);
}
for (kasp = ISC_LIST_HEAD(kasplist); kasp != NULL;
kasp = kasp_next)
{

View file

@ -78,7 +78,7 @@ cfg_keystore_fromconfig(const cfg_obj_t *config, isc_mem_t *mctx,
*
*\li 'logctx' is a valid logging context.
*
*\li kspp != NULL && *kspp == NULL
*\li kspp == NULL || *kspp == NULL
*
* Returns:
*

View file

@ -737,8 +737,6 @@ cfg_keystore_fromconfig(const cfg_obj_t *config, isc_mem_t *mctx,
dns_keystore_t *keystore = NULL;
int i = 0;
REQUIRE(kspp != NULL && *kspp == NULL);
if (config != NULL) {
name = cfg_obj_asstring(cfg_tuple_get(config, "name"));
} else {
@ -789,7 +787,10 @@ cfg_keystore_fromconfig(const cfg_obj_t *config, isc_mem_t *mctx,
INSIST(!(ISC_LIST_EMPTY(*keystorelist)));
/* Success: Attach the keystore to the pointer and return. */
dns_keystore_attach(keystore, kspp);
if (kspp != NULL) {
INSIST(*kspp == NULL);
dns_keystore_attach(keystore, kspp);
}
/* Don't detach as keystore is on '*keystorelist' */
return (ISC_R_SUCCESS);