diff --git a/CHANGES b/CHANGES index 5f6e79aaed..6b10bb78e3 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +4940. [cleanup] Extract the loop in dns__zone_updatesigs() into + separate functions to improve code readability. + [GL #135] + 4939. [test] Add basic unit tests for update_sigs(). [GL #135] 4938. [placeholder] diff --git a/lib/dns/diff.c b/lib/dns/diff.c index fe7181b773..7ed4faeba5 100644 --- a/lib/dns/diff.c +++ b/lib/dns/diff.c @@ -196,8 +196,6 @@ dns_diff_appendminimal(dns_diff_t *diff, dns_difftuple_t **tuplep) ISC_LIST_APPEND(diff->tuples, *tuplep, link); *tuplep = NULL; } - - ENSURE(*tuplep == NULL); } static isc_stdtime_t diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 568e4727f7..9274cd1d28 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -7288,6 +7288,40 @@ need_nsec_chain(dns_db_t *db, dns_dbversion_t *ver, return (result); } +/*% + * Given a tuple which is part of a diff, return a pointer to the next tuple in + * that diff which has the same name and type (or NULL if no such tuple is + * found). + */ +static dns_difftuple_t * +find_next_matching_tuple(dns_difftuple_t *cur) { + dns_difftuple_t *next = cur; + + while ((next = ISC_LIST_NEXT(next, link)) != NULL) { + if (cur->rdata.type == next->rdata.type && + dns_name_equal(&cur->name, &next->name)) + { + return (next); + } + } + + return (NULL); +} + +/*% + * Remove all tuples with the same name and type as 'cur' from 'src' and append + * them to 'dst'. + */ +static void +move_matching_tuples(dns_difftuple_t *cur, dns_diff_t *src, dns_diff_t *dst) { + do { + dns_difftuple_t *next = find_next_matching_tuple(cur); + ISC_LIST_UNLINK(src->tuples, cur, link); + dns_diff_appendminimal(dst, &cur); + cur = next; + } while (cur != NULL); +} + /*% * Add/remove DNSSEC signatures for the list of "raw" zone changes supplied in * 'diff'. Gradually remove tuples from 'diff' and append them to 'zonediff' @@ -7304,9 +7338,7 @@ dns__zone_updatesigs(dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *version, dns_difftuple_t *tuple; isc_result_t result; - for (tuple = ISC_LIST_HEAD(diff->tuples); - tuple != NULL; - tuple = ISC_LIST_HEAD(diff->tuples)) { + while ((tuple = ISC_LIST_HEAD(diff->tuples)) != NULL) { isc_stdtime_t exp = expire; if (keyexpire != 0 && @@ -7337,17 +7369,14 @@ dns__zone_updatesigs(dns_diff_t *diff, dns_db_t *db, dns_dbversion_t *version, return (result); } - do { - dns_difftuple_t *next = ISC_LIST_NEXT(tuple, link); - while (next != NULL && - (tuple->rdata.type != next->rdata.type || - !dns_name_equal(&tuple->name, &next->name))) - next = ISC_LIST_NEXT(next, link); - ISC_LIST_UNLINK(diff->tuples, tuple, link); - dns_diff_appendminimal(zonediff->diff, &tuple); - INSIST(tuple == NULL); - tuple = next; - } while (tuple != NULL); + /* + * Signature changes for all RRs with name tuple->name and type + * tuple->rdata.type were appended to zonediff->diff. Now we + * remove all the "raw" changes with the same name and type + * from diff (so that they are not processed by this loop + * again) and append them to zonediff so that they get applied. + */ + move_matching_tuples(tuple, diff, zonediff->diff); } return (ISC_R_SUCCESS); }