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.

diff --git a/bin/dnssec/dnssec-keygen.c b/bin/dnssec/dnssec-keygen.c
index b9e5fdb62bcfafe1c564fe5170ed8335f1d3ad69..7876f3f731cfd06ffd1a4b2b61b990a014363ee9 100644 (file)
--- a/bin/dnssec/dnssec-keygen.c
+++ b/bin/dnssec/dnssec-keygen.c
@@ -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;

diff --git a/bin/named/server.c b/bin/named/server.c
index 64fbd5dc99b86968a1b4ddc23c04a76f35af503c..d2da295f06e65c361941f5cef2006144ea72dfcd 100644 (file)
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -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);
        }
 
        /*

diff --git a/lib/dns/include/dns/keystore.h b/lib/dns/include/dns/keystore.h
index 8cf573cb31898a977130a54f2dc6c92eb8c9a00d..8888eb12d46d56e103a5745e39d6f439a85562e0 100644 (file)
--- a/lib/dns/include/dns/keystore.h
+++ b/lib/dns/include/dns/keystore.h
@@ -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

diff --git a/lib/dns/keystore.c b/lib/dns/keystore.c
index 128bededc06e80b0f8ed09e8ac2d67211235f34f..90b766d62e264c02e13345aa26400ff531304ce4 100644 (file)
--- a/lib/dns/keystore.c
+++ b/lib/dns/keystore.c
@@ -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) {

diff --git a/lib/isccfg/check.c b/lib/isccfg/check.c
index ad4ae8441d3622a4533449edb1267ef5687a1e87..c4ddf1940dc1942b10b282ce4d6fe18152f188b8 100644 (file)
--- a/lib/isccfg/check.c
+++ b/lib/isccfg/check.c
@@ -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)
                {

diff --git a/lib/isccfg/include/isccfg/kaspconf.h b/lib/isccfg/include/isccfg/kaspconf.h
index 1bc727d89c6b4885a65730101b29526627aa22a0..ccc1cecc5d9d49208a96b97bd1b839b12382f88d 100644 (file)
--- a/lib/isccfg/include/isccfg/kaspconf.h
+++ b/lib/isccfg/include/isccfg/kaspconf.h
@@ -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:
  *

diff --git a/lib/isccfg/kaspconf.c b/lib/isccfg/kaspconf.c
index 583f482f106fa496307dcc0ed5b765d32afa2fcb..f756ed97dae69ac5a25dc41c70e9a5c54d2b77b3 100644 (file)
--- a/lib/isccfg/kaspconf.c
+++ b/lib/isccfg/kaspconf.c
@@ -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);