fix dns_qp_insert() checks in qpzone

in some places there were checks for failures of dns_qp_insert()
after dns_qp_getname(). such failures could only happen if another
thread inserted a node between the two calls, and that can't happen
because the calls are serialized with dns_qpmulti_write(). we can
simplify the code and just add an INSIST.

diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c
index 14ede511ab50d78bf760613f109b57611eba45c6..e772ee11be70aaaf58463b6e3a96a21011d70717 100644 (file)
--- a/lib/dns/qpzone.c
+++ b/lib/dns/qpzone.c
@@ -703,15 +703,10 @@ dns__qpzone_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type,
        dns_qpmulti_write(qpdb->tree, &qp);
        qpdb->origin = new_qpznode(qpdb, &qpdb->common.origin);
        result = dns_qp_insert(qp, qpdb->origin, 0);
+       INSIST(result == ISC_R_SUCCESS);
        qpdb->origin->nsec = DNS_DB_NSEC_NORMAL;
        dns_qpmulti_commit(qpdb->tree, &qp);
 
-       if (result != ISC_R_SUCCESS) {
-               INSIST(result != ISC_R_EXISTS);
-               qpzonedb_detach(&qpdb);
-               return result;
-       }
-
        /*
         * Add an apex node to the NSEC3 tree so that NSEC3 searches
         * return partial matches when there is only a single NSEC3
@@ -721,14 +716,9 @@ dns__qpzone_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type,
        qpdb->nsec3_origin = new_qpznode(qpdb, &qpdb->common.origin);
        qpdb->nsec3_origin->nsec = DNS_DB_NSEC_NSEC3;
        result = dns_qp_insert(qp, qpdb->nsec3_origin, 0);
+       INSIST(result == ISC_R_SUCCESS);
        dns_qpmulti_commit(qpdb->nsec3, &qp);
 
-       if (result != ISC_R_SUCCESS) {
-               INSIST(result != ISC_R_EXISTS);
-               qpzonedb_detach(&qpdb);
-               return result;
-       }
-
        /*
         * Keep the current version in the open list so that list operation
         * won't happen in normal lookup operations.
@@ -1700,13 +1690,14 @@ loading_addnode(qpz_load_t *loadctx, const dns_name_t *name,
         * We're adding an NSEC record, so create a node in the nsec tree
         * too. This tree speeds searches for closest NSECs that would
         * otherwise need to examine many irrelevant nodes in large TLDs.
+        * If dns_qp_insert() fails, it means there's already an NSEC
+        * node there, so we can just detach the new one we created and
+        * move on.
         */
-       nsecnode = new_qpznode(qpdb, name);
-       result = dns_qp_insert(loadctx->nsec, nsecnode, 0);
        node->nsec = DNS_DB_NSEC_HAS_NSEC;
-       if (result == ISC_R_SUCCESS) {
-               nsecnode->nsec = DNS_DB_NSEC_NSEC;
-       }
+       nsecnode = new_qpznode(qpdb, name);
+       nsecnode->nsec = DNS_DB_NSEC_NSEC;
+       (void)dns_qp_insert(loadctx->nsec, nsecnode, 0);
        qpznode_detach(&nsecnode);
 
 done:
@@ -2537,22 +2528,21 @@ findnodeintree(qpzonedb_t *qpdb, const dns_name_t *name, bool create,
 
                node = new_qpznode(qpdb, name);
                result = dns_qp_insert(qp, node, 0);
+               INSIST(result == ISC_R_SUCCESS);
                qpznode_unref(node);
 
-               if (result == ISC_R_SUCCESS) {
-                       if (nsec3) {
-                               node->nsec = DNS_DB_NSEC_NSEC3;
-                       } else {
-                               addwildcards(qpdb, qp, name);
-                               if (dns_name_iswildcard(name)) {
-                                       wildcardmagic(qpdb, qp, name);
-                               }
+               if (nsec3) {
+                       node->nsec = DNS_DB_NSEC_NSEC3;
+               } else {
+                       addwildcards(qpdb, qp, name);
+                       if (dns_name_iswildcard(name)) {
+                               wildcardmagic(qpdb, qp, name);
                        }
                }
-
-               INSIST(node->nsec == DNS_DB_NSEC_NSEC3 || !nsec3);
        }
 
+       INSIST(node->nsec == DNS_DB_NSEC_NSEC3 || !nsec3);
+
        qpznode_acquire(qpdb, node DNS__DB_FLARG_PASS);
 
        if (create) {
@@ -4730,15 +4720,16 @@ qpzone_addrdataset(dns_db_t *db, dns_dbnode_t *dbnode,
 
        result = ISC_R_SUCCESS;
        if (nsec != NULL) {
+               node->nsec = DNS_DB_NSEC_HAS_NSEC;
+
+               /*
+                * If it fails, there was already an NSEC node,
+                * so we can detach the new one we created and
+                * move on.
+                */
                qpznode_t *nsecnode = new_qpznode(qpdb, name);
-               result = dns_qp_insert(nsec, nsecnode, 0);
-               if (result == ISC_R_SUCCESS) {
-                       nsecnode->nsec = DNS_DB_NSEC_NSEC;
-                       node->nsec = DNS_DB_NSEC_HAS_NSEC;
-               } else if (result == ISC_R_EXISTS) {
-                       node->nsec = DNS_DB_NSEC_HAS_NSEC;
-                       result = ISC_R_SUCCESS;
-               }
+               nsecnode->nsec = DNS_DB_NSEC_NSEC;
+               (void)dns_qp_insert(nsec, nsecnode, 0);
                qpznode_detach(&nsecnode);
        }