]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Replace SAVE/RESTORE/INITANDSAVE macros with MOVE_OWNERSHIP()
authorOndřej Surý <ondrej@isc.org>
Sat, 21 Mar 2026 14:35:34 +0000 (15:35 +0100)
committerOndřej Surý <ondrej@isc.org>
Mon, 23 Mar 2026 10:06:28 +0000 (11:06 +0100)
Replace the local SAVE(), RESTORE(), and INITANDSAVE() macros in
query.c with the project-wide MOVE_OWNERSHIP() macro.  The new
form is clearer about the intent: ownership of a pointer is being
transferred from source to destination, with the source set to NULL.

SAVE and RESTORE were identical macros with different names used to
indicate the direction of transfer, but this distinction was purely
cosmetic.  INITANDSAVE additionally set the destination to NULL
first, which is unnecessary because the preceding memcpy already
initialized all fields from the source struct.

lib/ns/query.c

index 8e7891d60c07c4cdb5537fcdbcd6b4436376b3e9..3774d4dea7916f6d7b0558a8afa555d69be44bd6 100644 (file)
@@ -190,25 +190,6 @@ client_trace(ns_client_t *client, int level, const char *message) {
 
 #define SFCACHE_CDFLAG 0x1
 
-/*
- * SAVE and RESTORE have the same semantics as:
- *
- *     foo_attach(b, &a);
- *     foo_detach(&b);
- *
- * without the locking and magic testing.
- *
- * We use the names SAVE and RESTORE to show the operation being performed,
- * even though the two macros are identical.
- */
-#define SAVE(a, b)                 \
-       do {                       \
-               INSIST(a == NULL); \
-               a = b;             \
-               b = NULL;          \
-       } while (0)
-#define RESTORE(a, b) SAVE(a, b)
-
 static atomic_uint_fast32_t last_rpznotready_log = 0;
 
 static bool
@@ -3104,11 +3085,11 @@ rpz_rrset_find(ns_client_t *client, dns_name_t *name, dns_rdatatype_t type,
                INSIST(*rdatasetp == NULL ||
                       !dns_rdataset_isassociated(*rdatasetp));
                st->state &= ~DNS_RPZ_RECURSING;
-               RESTORE(*dbp, st->r.db);
+               *dbp = MOVE_OWNERSHIP(st->r.db);
                if (*rdatasetp != NULL) {
                        ns_client_putrdataset(client, rdatasetp);
                }
-               RESTORE(*rdatasetp, st->r.r_rdataset);
+               *rdatasetp = MOVE_OWNERSHIP(st->r.r_rdataset);
                result = st->r.r_result;
                if (result == DNS_R_DELEGATION) {
                        CTRACE(ISC_LOG_ERROR, "RPZ recursing");
@@ -3437,22 +3418,22 @@ rpz_save_p(dns_rpz_st_t *st, dns_rpz_zone_t *rpz, dns_rpz_type_t rpz_type,
        dns_name_copy(p_name, st->p_name);
        st->m.prefix = prefix;
        st->m.result = result;
-       SAVE(st->m.zone, *zonep);
-       SAVE(st->m.db, *dbp);
-       SAVE(st->m.node, *nodep);
+       st->m.zone = MOVE_OWNERSHIP(*zonep);
+       st->m.db = MOVE_OWNERSHIP(*dbp);
+       st->m.node = MOVE_OWNERSHIP(*nodep);
        if (*rdatasetp != NULL && dns_rdataset_isassociated(*rdatasetp)) {
                /*
                 * Save the replacement rdataset from the policy
                 * and make the previous replacement rdataset scratch.
                 */
-               SAVE(trdataset, st->m.rdataset);
-               SAVE(st->m.rdataset, *rdatasetp);
-               SAVE(*rdatasetp, trdataset);
+               trdataset = MOVE_OWNERSHIP(st->m.rdataset);
+               st->m.rdataset = MOVE_OWNERSHIP(*rdatasetp);
+               *rdatasetp = MOVE_OWNERSHIP(trdataset);
                st->m.ttl = ISC_MIN(st->m.rdataset->ttl, rpz->max_policy_ttl);
        } else {
                st->m.ttl = ISC_MIN(DNS_RPZ_TTL_DEFAULT, rpz->max_policy_ttl);
        }
-       SAVE(st->m.version, version);
+       st->m.version = MOVE_OWNERSHIP(version);
 }
 
 /*
@@ -4696,7 +4677,8 @@ dns64_aaaaok(ns_client_t *client, dns_rdataset_t *rdataset,
        {
                for (i = 0; i < count; i++) {
                        if (aaaaok != NULL && !aaaaok[i]) {
-                               SAVE(client->query.dns64_aaaaok, aaaaok);
+                               client->query.dns64_aaaaok =
+                                       MOVE_OWNERSHIP(aaaaok);
                                client->query.dns64_aaaaoklen = count;
                                break;
                        }
@@ -5121,15 +5103,6 @@ qctx_deinit(query_ctx_t *qctx) {
        }
 }
 
-/*
- * Call SAVE but set 'a' to NULL first so as not to assert.
- */
-#define INITANDSAVE(a, b)   \
-       do {                \
-               a = NULL;   \
-               SAVE(a, b); \
-       } while (0)
-
 /*
  * "save" qctx data from 'src' to 'tgt'.
  * It essentially moves ownership of the data from src to tgt, so the former
@@ -5148,24 +5121,24 @@ qctx_save(query_ctx_t *src, query_ctx_t **targetp) {
        *target = *src;
 
        /* Then "move" pointers (except client and view) */
-       INITANDSAVE(target->dbuf, src->dbuf);
-       INITANDSAVE(target->fname, src->fname);
-       INITANDSAVE(target->tname, src->tname);
-       INITANDSAVE(target->rdataset, src->rdataset);
-       INITANDSAVE(target->sigrdataset, src->sigrdataset);
-       INITANDSAVE(target->noqname, src->noqname);
-       INITANDSAVE(target->fresp, src->fresp);
-       INITANDSAVE(target->db, src->db);
-       INITANDSAVE(target->version, src->version);
-       INITANDSAVE(target->node, src->node);
-       INITANDSAVE(target->zdb, src->zdb);
-       INITANDSAVE(target->znode, src->znode);
-       INITANDSAVE(target->zfname, src->zfname);
-       INITANDSAVE(target->zversion, src->zversion);
-       INITANDSAVE(target->zrdataset, src->zrdataset);
-       INITANDSAVE(target->zsigrdataset, src->zsigrdataset);
-       INITANDSAVE(target->rpz_st, src->rpz_st);
-       INITANDSAVE(target->zone, src->zone);
+       target->dbuf = MOVE_OWNERSHIP(src->dbuf);
+       target->fname = MOVE_OWNERSHIP(src->fname);
+       target->tname = MOVE_OWNERSHIP(src->tname);
+       target->rdataset = MOVE_OWNERSHIP(src->rdataset);
+       target->sigrdataset = MOVE_OWNERSHIP(src->sigrdataset);
+       target->noqname = MOVE_OWNERSHIP(src->noqname);
+       target->fresp = MOVE_OWNERSHIP(src->fresp);
+       target->db = MOVE_OWNERSHIP(src->db);
+       target->version = MOVE_OWNERSHIP(src->version);
+       target->node = MOVE_OWNERSHIP(src->node);
+       target->zdb = MOVE_OWNERSHIP(src->zdb);
+       target->znode = MOVE_OWNERSHIP(src->znode);
+       target->zfname = MOVE_OWNERSHIP(src->zfname);
+       target->zversion = MOVE_OWNERSHIP(src->zversion);
+       target->zrdataset = MOVE_OWNERSHIP(src->zrdataset);
+       target->zsigrdataset = MOVE_OWNERSHIP(src->zsigrdataset);
+       target->rpz_st = MOVE_OWNERSHIP(src->rpz_st);
+       target->zone = MOVE_OWNERSHIP(src->zone);
 
        /* View has to stay in 'src' for qctx_destroy. */
        target->view = NULL;
@@ -5515,10 +5488,9 @@ ns__query_start(query_ctx_t *qctx) {
                                dns_zone_detach(&qctx->zone);
                                qctx->zhooks = NULL;
                        }
-                       qctx->version = NULL;
-                       RESTORE(qctx->version, tversion);
-                       RESTORE(qctx->db, tdb);
-                       RESTORE(qctx->zone, tzone);
+                       qctx->version = MOVE_OWNERSHIP(tversion);
+                       qctx->db = MOVE_OWNERSHIP(tdb);
+                       qctx->zone = MOVE_OWNERSHIP(tzone);
                        qctx->is_zone = true;
                        result = ISC_R_SUCCESS;
                } else {
@@ -6088,7 +6060,7 @@ fetch_callback(void *arg) {
        }
        UNLOCK(&client->query.fetchlock);
 
-       SAVE(fetch, resp->fetch);
+       fetch = MOVE_OWNERSHIP(resp->fetch);
 
        /*
         * We're done recursing, detach from quota and unlink from
@@ -6386,19 +6358,20 @@ query_resume(query_ctx_t *qctx) {
 
                qctx->is_zone = qctx->rpz_st->q.is_zone;
                qctx->authoritative = qctx->rpz_st->q.authoritative;
-               RESTORE(qctx->zone, qctx->rpz_st->q.zone);
-               RESTORE(qctx->node, qctx->rpz_st->q.node);
-               RESTORE(qctx->db, qctx->rpz_st->q.db);
-               RESTORE(qctx->rdataset, qctx->rpz_st->q.rdataset);
-               RESTORE(qctx->sigrdataset, qctx->rpz_st->q.sigrdataset);
+               qctx->zone = MOVE_OWNERSHIP(qctx->rpz_st->q.zone);
+               qctx->node = MOVE_OWNERSHIP(qctx->rpz_st->q.node);
+               qctx->db = MOVE_OWNERSHIP(qctx->rpz_st->q.db);
+               qctx->rdataset = MOVE_OWNERSHIP(qctx->rpz_st->q.rdataset);
+               qctx->sigrdataset = MOVE_OWNERSHIP(qctx->rpz_st->q.sigrdataset);
                qctx->qtype = qctx->rpz_st->q.qtype;
 
                if (qctx->fresp->node != NULL) {
                        dns_db_detachnode(&qctx->fresp->node);
                }
-               SAVE(qctx->rpz_st->r.db, qctx->fresp->cache);
+               qctx->rpz_st->r.db = MOVE_OWNERSHIP(qctx->fresp->cache);
                qctx->rpz_st->r.r_type = qctx->fresp->qtype;
-               SAVE(qctx->rpz_st->r.r_rdataset, qctx->fresp->rdataset);
+               qctx->rpz_st->r.r_rdataset =
+                       MOVE_OWNERSHIP(qctx->fresp->rdataset);
                ns_client_putrdataset(qctx->client, &qctx->fresp->sigrdataset);
        } else if (REDIRECT(qctx->client)) {
                /*
@@ -6417,12 +6390,13 @@ query_resume(query_ctx_t *qctx) {
 #endif /* ifdef WANT_QUERYTRACE */
                qctx->qtype = qctx->client->query.redirect.qtype;
                INSIST(qctx->client->query.redirect.rdataset != NULL);
-               RESTORE(qctx->rdataset, qctx->client->query.redirect.rdataset);
-               RESTORE(qctx->sigrdataset,
+               qctx->rdataset =
+                       MOVE_OWNERSHIP(qctx->client->query.redirect.rdataset);
+               qctx->sigrdataset = MOVE_OWNERSHIP(
                        qctx->client->query.redirect.sigrdataset);
-               RESTORE(qctx->db, qctx->client->query.redirect.db);
-               RESTORE(qctx->node, qctx->client->query.redirect.node);
-               RESTORE(qctx->zone, qctx->client->query.redirect.zone);
+               qctx->db = MOVE_OWNERSHIP(qctx->client->query.redirect.db);
+               qctx->node = MOVE_OWNERSHIP(qctx->client->query.redirect.node);
+               qctx->zone = MOVE_OWNERSHIP(qctx->client->query.redirect.zone);
                qctx->authoritative =
                        qctx->client->query.redirect.authoritative;
 
@@ -6442,10 +6416,10 @@ query_resume(query_ctx_t *qctx) {
                qctx->authoritative = false;
 
                qctx->qtype = qctx->fresp->qtype;
-               SAVE(qctx->db, qctx->fresp->cache);
-               SAVE(qctx->node, qctx->fresp->node);
-               SAVE(qctx->rdataset, qctx->fresp->rdataset);
-               SAVE(qctx->sigrdataset, qctx->fresp->sigrdataset);
+               qctx->db = MOVE_OWNERSHIP(qctx->fresp->cache);
+               qctx->node = MOVE_OWNERSHIP(qctx->fresp->node);
+               qctx->rdataset = MOVE_OWNERSHIP(qctx->fresp->rdataset);
+               qctx->sigrdataset = MOVE_OWNERSHIP(qctx->fresp->sigrdataset);
        }
        INSIST(qctx->rdataset != NULL);
 
@@ -6546,7 +6520,7 @@ query_hookresume(void *arg) {
                canceled = true;
        }
        UNLOCK(&client->query.fetchlock);
-       SAVE(hctx, rev->ctx);
+       hctx = MOVE_OWNERSHIP(rev->ctx);
 
        release_recursionquota(client);
 
@@ -7019,11 +6993,11 @@ query_checkrpz(query_ctx_t *qctx, isc_result_t result) {
                qctx->rpz_st->q.qtype = qctx->qtype;
                qctx->rpz_st->q.is_zone = qctx->is_zone;
                qctx->rpz_st->q.authoritative = qctx->authoritative;
-               SAVE(qctx->rpz_st->q.zone, qctx->zone);
-               SAVE(qctx->rpz_st->q.db, qctx->db);
-               SAVE(qctx->rpz_st->q.node, qctx->node);
-               SAVE(qctx->rpz_st->q.rdataset, qctx->rdataset);
-               SAVE(qctx->rpz_st->q.sigrdataset, qctx->sigrdataset);
+               qctx->rpz_st->q.zone = MOVE_OWNERSHIP(qctx->zone);
+               qctx->rpz_st->q.db = MOVE_OWNERSHIP(qctx->db);
+               qctx->rpz_st->q.node = MOVE_OWNERSHIP(qctx->node);
+               qctx->rpz_st->q.rdataset = MOVE_OWNERSHIP(qctx->rdataset);
+               qctx->rpz_st->q.sigrdataset = MOVE_OWNERSHIP(qctx->sigrdataset);
                dns_name_copy(qctx->fname, qctx->rpz_st->fname);
                qctx->rpz_st->q.result = result;
                qctx->client->query.attributes |= NS_QUERYATTR_RECURSING;
@@ -7053,16 +7027,17 @@ query_checkrpz(query_ctx_t *qctx, isc_result_t result) {
                rpz_clean(&qctx->zone, &qctx->db, &qctx->node, NULL);
                if (qctx->rpz_st->m.rdataset != NULL) {
                        ns_client_putrdataset(qctx->client, &qctx->rdataset);
-                       RESTORE(qctx->rdataset, qctx->rpz_st->m.rdataset);
+                       qctx->rdataset =
+                               MOVE_OWNERSHIP(qctx->rpz_st->m.rdataset);
                } else {
                        qctx_clean(qctx);
                }
                qctx->version = NULL;
 
-               RESTORE(qctx->node, qctx->rpz_st->m.node);
-               RESTORE(qctx->db, qctx->rpz_st->m.db);
-               RESTORE(qctx->version, qctx->rpz_st->m.version);
-               RESTORE(qctx->zone, qctx->rpz_st->m.zone);
+               qctx->node = MOVE_OWNERSHIP(qctx->rpz_st->m.node);
+               qctx->db = MOVE_OWNERSHIP(qctx->rpz_st->m.db);
+               qctx->version = MOVE_OWNERSHIP(qctx->rpz_st->m.version);
+               qctx->zone = MOVE_OWNERSHIP(qctx->rpz_st->m.zone);
 
                /*
                 * Add SOA record to additional section
@@ -8008,8 +7983,9 @@ query_respond(query_ctx_t *qctx) {
                 * Look to see if there are A records for this name.
                 */
                qctx->client->query.dns64_ttl = qctx->rdataset->ttl;
-               SAVE(qctx->client->query.dns64_aaaa, qctx->rdataset);
-               SAVE(qctx->client->query.dns64_sigaaaa, qctx->sigrdataset);
+               qctx->client->query.dns64_aaaa = MOVE_OWNERSHIP(qctx->rdataset);
+               qctx->client->query.dns64_sigaaaa =
+                       MOVE_OWNERSHIP(qctx->sigrdataset);
                ns_client_releasename(qctx->client, &qctx->fname);
                dns_db_detachnode(&qctx->node);
                qctx->type = qctx->qtype = dns_rdatatype_a;
@@ -8505,10 +8481,9 @@ query_zone_delegation(query_ctx_t *qctx) {
                                dns_zone_detach(&qctx->zone);
                                qctx->zhooks = NULL;
                        }
-                       qctx->version = NULL;
-                       RESTORE(qctx->version, tversion);
-                       RESTORE(qctx->db, tdb);
-                       RESTORE(qctx->zone, tzone);
+                       qctx->version = MOVE_OWNERSHIP(tversion);
+                       qctx->db = MOVE_OWNERSHIP(tdb);
+                       qctx->zone = MOVE_OWNERSHIP(tzone);
                        qctx->authoritative = true;
 
                        return query_lookup(qctx);
@@ -8530,12 +8505,12 @@ query_zone_delegation(query_ctx_t *qctx) {
                 * we'll restore these values there.
                 */
                ns_client_keepname(qctx->client, qctx->fname, qctx->dbuf);
-               SAVE(qctx->zdb, qctx->db);
-               SAVE(qctx->znode, qctx->node);
-               SAVE(qctx->zfname, qctx->fname);
-               SAVE(qctx->zversion, qctx->version);
-               SAVE(qctx->zrdataset, qctx->rdataset);
-               SAVE(qctx->zsigrdataset, qctx->sigrdataset);
+               qctx->zdb = MOVE_OWNERSHIP(qctx->db);
+               qctx->znode = MOVE_OWNERSHIP(qctx->node);
+               qctx->zfname = MOVE_OWNERSHIP(qctx->fname);
+               qctx->zversion = MOVE_OWNERSHIP(qctx->version);
+               qctx->zrdataset = MOVE_OWNERSHIP(qctx->rdataset);
+               qctx->zsigrdataset = MOVE_OWNERSHIP(qctx->sigrdataset);
                dns_db_attach(qctx->view->cachedb, &qctx->db);
                qctx->is_zone = false;
 
@@ -8623,12 +8598,12 @@ query_delegation(query_ctx_t *qctx) {
 
                dns_db_detachnode(&qctx->node);
                dns_db_detach(&qctx->db);
-               RESTORE(qctx->db, qctx->zdb);
-               RESTORE(qctx->node, qctx->znode);
-               RESTORE(qctx->fname, qctx->zfname);
-               RESTORE(qctx->version, qctx->zversion);
-               RESTORE(qctx->rdataset, qctx->zrdataset);
-               RESTORE(qctx->sigrdataset, qctx->zsigrdataset);
+               qctx->db = MOVE_OWNERSHIP(qctx->zdb);
+               qctx->node = MOVE_OWNERSHIP(qctx->znode);
+               qctx->fname = MOVE_OWNERSHIP(qctx->zfname);
+               qctx->version = MOVE_OWNERSHIP(qctx->zversion);
+               qctx->rdataset = MOVE_OWNERSHIP(qctx->zrdataset);
+               qctx->sigrdataset = MOVE_OWNERSHIP(qctx->zsigrdataset);
        }
 
        result = query_delegation_recurse(qctx);
@@ -8872,8 +8847,9 @@ query_nodata(query_ctx_t *qctx, isc_result_t res) {
                if (qctx->sigrdataset != NULL) {
                        ns_client_putrdataset(qctx->client, &qctx->sigrdataset);
                }
-               RESTORE(qctx->rdataset, qctx->client->query.dns64_aaaa);
-               RESTORE(qctx->sigrdataset, qctx->client->query.dns64_sigaaaa);
+               qctx->rdataset = MOVE_OWNERSHIP(qctx->client->query.dns64_aaaa);
+               qctx->sigrdataset =
+                       MOVE_OWNERSHIP(qctx->client->query.dns64_sigaaaa);
                if (qctx->fname == NULL) {
                        qctx->dbuf = ns_client_getnamebuf(qctx->client);
                        qctx->fname = ns_client_newname(qctx->client,
@@ -8923,8 +8899,9 @@ query_nodata(query_ctx_t *qctx, isc_result_t res) {
                        UNREACHABLE();
                }
 
-               SAVE(qctx->client->query.dns64_aaaa, qctx->rdataset);
-               SAVE(qctx->client->query.dns64_sigaaaa, qctx->sigrdataset);
+               qctx->client->query.dns64_aaaa = MOVE_OWNERSHIP(qctx->rdataset);
+               qctx->client->query.dns64_sigaaaa =
+                       MOVE_OWNERSHIP(qctx->sigrdataset);
                ns_client_releasename(qctx->client, &qctx->fname);
                dns_db_detachnode(&qctx->node);
                qctx->type = qctx->qtype = dns_rdatatype_a;
@@ -9270,14 +9247,15 @@ query_redirect(query_ctx_t *qctx, isc_result_t saved_result) {
        case DNS_R_CONTINUE:
                inc_stats(qctx->client,
                          ns_statscounter_nxdomainredirect_rlookup);
-               SAVE(qctx->client->query.redirect.db, qctx->db);
-               SAVE(qctx->client->query.redirect.node, qctx->node);
-               SAVE(qctx->client->query.redirect.zone, qctx->zone);
+               qctx->client->query.redirect.db = MOVE_OWNERSHIP(qctx->db);
+               qctx->client->query.redirect.node = MOVE_OWNERSHIP(qctx->node);
+               qctx->client->query.redirect.zone = MOVE_OWNERSHIP(qctx->zone);
                qctx->client->query.redirect.qtype = qctx->qtype;
                INSIST(qctx->rdataset != NULL);
-               SAVE(qctx->client->query.redirect.rdataset, qctx->rdataset);
-               SAVE(qctx->client->query.redirect.sigrdataset,
-                    qctx->sigrdataset);
+               qctx->client->query.redirect.rdataset =
+                       MOVE_OWNERSHIP(qctx->rdataset);
+               qctx->client->query.redirect.sigrdataset =
+                       MOVE_OWNERSHIP(qctx->sigrdataset);
                qctx->client->query.redirect.result = saved_result;
                dns_name_copy(qctx->fname, qctx->client->query.redirect.fname);
                qctx->client->query.redirect.authoritative =
@@ -10679,10 +10657,10 @@ db_find:
                if (USECACHE(client)) {
                        ns_client_keepname(client, fname, dbuf);
                        dns_db_detachnode(&node);
-                       SAVE(zdb, db);
-                       SAVE(zfname, fname);
-                       SAVE(zrdataset, rdataset);
-                       SAVE(zsigrdataset, sigrdataset);
+                       zdb = MOVE_OWNERSHIP(db);
+                       zfname = MOVE_OWNERSHIP(fname);
+                       zrdataset = MOVE_OWNERSHIP(rdataset);
+                       zsigrdataset = MOVE_OWNERSHIP(sigrdataset);
                        version = NULL;
                        dns_db_attach(client->inner.view->cachedb, &db);
                        is_zone = false;
@@ -10733,10 +10711,10 @@ db_find:
                }
                dns_db_detach(&db);
 
-               RESTORE(db, zdb);
-               RESTORE(fname, zfname);
-               RESTORE(rdataset, zrdataset);
-               RESTORE(sigrdataset, zsigrdataset);
+               db = MOVE_OWNERSHIP(zdb);
+               fname = MOVE_OWNERSHIP(zfname);
+               rdataset = MOVE_OWNERSHIP(zrdataset);
+               sigrdataset = MOVE_OWNERSHIP(zsigrdataset);
        }
 
        /*