Merge pull request #23289 from yuwata/resolve-answer-add-rrsig

resolve: place RRSIG after the corresponding entries

src/basic/ordered-set.h

@@ -74,6 +74,10 @@ static inline char** ordered_set_get_strv(OrderedSet *s) {
         return _hashmap_get_strv(HASHMAP_BASE((OrderedHashmap*) s));
 }
 
+static inline int ordered_set_reserve(OrderedSet *s, unsigned entries_add) {
+        return ordered_hashmap_reserve((OrderedHashmap*) s, entries_add);
+}
+
 int ordered_set_consume(OrderedSet *s, void *p);
 int _ordered_set_put_strdup(OrderedSet **s, const char *p  HASHMAP_DEBUG_PARAMS);
 #define ordered_set_put_strdup(s, p) _ordered_set_put_strdup(s, p  HASHMAP_DEBUG_SRC_ARGS)

src/resolve/resolved-dns-answer.c

@@ -9,6 +9,19 @@
 #include "resolved-dns-dnssec.h"
 #include "string-util.h"
 
+static DnsAnswerItem *dns_answer_item_free(DnsAnswerItem *item) {
+        if (!item)
+                return NULL;
+
+        dns_resource_record_unref(item->rr);
+        dns_resource_record_unref(item->rrsig);
+
+        return mfree(item);
+}
+
+DEFINE_PRIVATE_TRIVIAL_REF_UNREF_FUNC(DnsAnswerItem, dns_answer_item, dns_answer_item_free);
+DEFINE_TRIVIAL_CLEANUP_FUNC(DnsAnswerItem*, dns_answer_item_unref);
+
 static void dns_answer_item_hash_func(const DnsAnswerItem *a, struct siphash *state) {
         assert(a);
         assert(state);
@@ -31,54 +44,62 @@ static int dns_answer_item_compare_func(const DnsAnswerItem *a, const DnsAnswerI
         return dns_resource_record_compare_func(a->rr, b->rr);
 }
 
-DEFINE_PRIVATE_HASH_OPS(dns_answer_item_hash_ops, DnsAnswerItem, dns_answer_item_hash_func, dns_answer_item_compare_func);
+DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR(
+        dns_answer_item_hash_ops,
+        DnsAnswerItem,
+        dns_answer_item_hash_func,
+        dns_answer_item_compare_func,
+        dns_answer_item_unref);
 
-DnsAnswer *dns_answer_new(size_t n) {
+static int dns_answer_reserve_internal(DnsAnswer *a, size_t n) {
-        _cleanup_set_free_ Set *s = NULL;
+        size_t m;
-        DnsAnswer *a;
 
-        if (n > UINT16_MAX) /* We can only place 64K RRs in an answer at max */
+        assert(a);
+        assert(a->items);
+
+        m = ordered_set_size(a->items);
+        assert(m <= UINT16_MAX); /* We can only place 64K RRs in an answer at max */
+
+        if (n > UINT16_MAX - m)
                 n = UINT16_MAX;
-
+        else
-        s = set_new(&dns_answer_item_hash_ops);
+                n += m;
-        if (!s)
-                return NULL;
 
         /* Higher multipliers give slightly higher efficiency through hash collisions, but the gains
          * quickly drop off after 2. */
-        if (set_reserve(s, n * 2) < 0)
+        return ordered_set_reserve(a->items, n * 2);
-                return NULL;
-
-        a = malloc0(offsetof(DnsAnswer, items) + sizeof(DnsAnswerItem) * n);
-        if (!a)
-                return NULL;
-
-        a->n_ref = 1;
-        a->n_allocated = n;
-        a->set_items = TAKE_PTR(s);
-        return a;
 }
 
-static void dns_answer_flush(DnsAnswer *a) {
+DnsAnswer *dns_answer_new(size_t n) {
-        DnsAnswerItem *item;
+        _cleanup_ordered_set_free_ OrderedSet *s = NULL;
+        DnsAnswer *a;
 
+        if (n > UINT16_MAX)
+                n = UINT16_MAX;
+
+        s = ordered_set_new(&dns_answer_item_hash_ops);
+        if (!s)
+                return NULL;
+
+        a = new(DnsAnswer, 1);
         if (!a)
-                return;
+                return NULL;
 
-        a->set_items = set_free(a->set_items);
+        *a = (DnsAnswer) {
+                .n_ref = 1,
+                .items = TAKE_PTR(s),
+        };
 
-        DNS_ANSWER_FOREACH_ITEM(item, a) {
+        if (dns_answer_reserve_internal(a, n) < 0)
-                dns_resource_record_unref(item->rr);
+                return NULL;
-                dns_resource_record_unref(item->rrsig);
-        }
 
-        a->n_rrs = 0;
+        return a;
 }
 
 static DnsAnswer *dns_answer_free(DnsAnswer *a) {
         assert(a);
 
-        dns_answer_flush(a);
+        ordered_set_free(a->items);
         return mfree(a);
 }
 
@@ -91,6 +112,7 @@ static int dns_answer_add_raw(
                 DnsAnswerFlags flags,
                 DnsResourceRecord *rrsig) {
 
+        _cleanup_(dns_answer_item_unrefp) DnsAnswerItem *item = NULL;
         int r;
 
         assert(rr);
@@ -98,25 +120,26 @@ static int dns_answer_add_raw(
         if (!a)
                 return -ENOSPC;
 
-        if (a->n_rrs >= a->n_allocated)
+        if (dns_answer_size(a) >= UINT16_MAX)
                 return -ENOSPC;
 
-        a->items[a->n_rrs] = (DnsAnswerItem) {
+        item = new(DnsAnswerItem, 1);
-                .rr = rr,
+        if (!item)
+                return -ENOMEM;
+
+        *item = (DnsAnswerItem) {
+                .n_ref = 1,
+                .rr = dns_resource_record_ref(rr),
                 .ifindex = ifindex,
                 .flags = flags,
                 .rrsig = dns_resource_record_ref(rrsig),
         };
 
-        r = set_put(a->set_items, &a->items[a->n_rrs]);
+        r = ordered_set_put(a->items, item);
         if (r < 0)
                 return r;
-        if (r == 0)
-                return -EEXIST;
-
-        dns_resource_record_ref(rr);
-        a->n_rrs++;
 
+        TAKE_PTR(item);
         return 1;
 }
 
@@ -159,7 +182,7 @@ int dns_answer_add(
                 .ifindex = ifindex,
         };
 
-        exist = set_get(a->set_items, &tmp);
+        exist = ordered_set_get(a->items, &tmp);
         if (exist) {
                 /* There's already an RR of the same RRset in place! Let's see if the TTLs more or less
                  * match. We don't really care if they match precisely, but we do care whether one is 0 and
@@ -169,8 +192,9 @@ int dns_answer_add(
 
                 /* Entry already exists, keep the entry with the higher TTL. */
                 if (rr->ttl > exist->rr->ttl) {
+                        dns_resource_record_ref(rr);
                         dns_resource_record_unref(exist->rr);
-                        exist->rr = dns_resource_record_ref(rr); /* lgtm [cpp/inconsistent-null-check] */
+                        exist->rr = rr;
 
                         /* Update RRSIG and RR at the same time */
                         if (rrsig) {
@@ -181,6 +205,12 @@ int dns_answer_add(
                 }
 
                 exist->flags |= flags;
+
+                if (rr->key->type == DNS_TYPE_RRSIG) {
+                        /* If the rr is RRSIG, then move the rr to the end. */
+                        assert_se(ordered_set_remove(a->items, exist) == exist);
+                        assert_se(ordered_set_put(a->items, exist) == 1);
+                }
                 return 0;
         }
 
@@ -436,7 +466,7 @@ int dns_answer_merge(DnsAnswer *a, DnsAnswer *b, DnsAnswer **ret) {
                 return 0;
         }
 
-        k = dns_answer_new(a->n_rrs + b->n_rrs);
+        k = dns_answer_new(dns_answer_size(a) + dns_answer_size(b));
         if (!k)
                 return -ENOMEM;
 
@@ -470,9 +500,8 @@ int dns_answer_extend(DnsAnswer **a, DnsAnswer *b) {
 }
 
 int dns_answer_remove_by_key(DnsAnswer **a, const DnsResourceKey *key) {
-        bool found = false, other = false;
+        DnsAnswerItem *item;
-        DnsResourceRecord *rr;
+        bool found = false;
-        size_t i;
         int r;
 
         assert(a);
@@ -480,162 +509,51 @@ int dns_answer_remove_by_key(DnsAnswer **a, const DnsResourceKey *key) {
 
         /* Remove all entries matching the specified key from *a */
 
-        DNS_ANSWER_FOREACH(rr, *a) {
+        DNS_ANSWER_FOREACH_ITEM(item, *a) {
-                r = dns_resource_key_equal(rr->key, key);
+                r = dns_resource_key_equal(item->rr->key, key);
                 if (r < 0)
                         return r;
-                if (r > 0)
+                if (r > 0) {
+                        dns_answer_item_unref(ordered_set_remove((*a)->items, item));
                         found = true;
-                else
+                }
-                        other = true;
-
-                if (found && other)
-                        break;
         }
 
         if (!found)
                 return 0;
 
-        if (!other) {
+        if (dns_answer_isempty(*a))
                 *a = dns_answer_unref(*a); /* Return NULL for the empty answer */
-                return 1;
-        }
-
-        if ((*a)->n_ref > 1) {
-                _cleanup_(dns_answer_unrefp) DnsAnswer *copy = NULL;
-                DnsAnswerItem *item;
-
-                copy = dns_answer_new((*a)->n_rrs);
-                if (!copy)
-                        return -ENOMEM;
-
-                DNS_ANSWER_FOREACH_ITEM(item, *a) {
-                        r = dns_resource_key_equal(item->rr->key, key);
-                        if (r < 0)
-                                return r;
-                        if (r > 0)
-                                continue;
-
-                        r = dns_answer_add_raw(copy, item->rr, item->ifindex, item->flags, item->rrsig);
-                        if (r < 0)
-                                return r;
-                }
-
-                dns_answer_unref(*a);
-                *a = TAKE_PTR(copy);
-
-                return 1;
-        }
-
-        /* Only a single reference, edit in-place */
-
-        i = 0;
-        for (;;) {
-                if (i >= (*a)->n_rrs)
-                        break;
-
-                r = dns_resource_key_equal((*a)->items[i].rr->key, key);
-                if (r < 0)
-                        return r;
-                if (r > 0) {
-                        /* Kill this entry */
-
-                        dns_resource_record_unref((*a)->items[i].rr);
-                        dns_resource_record_unref((*a)->items[i].rrsig);
-
-                        memmove((*a)->items + i, (*a)->items + i + 1, sizeof(DnsAnswerItem) * ((*a)->n_rrs - i - 1));
-                        (*a)->n_rrs--;
-                        continue;
-
-                } else
-                        /* Keep this entry */
-                        i++;
-        }
 
         return 1;
 }
 
-int dns_answer_remove_by_rr(DnsAnswer **a, DnsResourceRecord *rm) {
+int dns_answer_remove_by_rr(DnsAnswer **a, DnsResourceRecord *rr) {
-        bool found = false, other = false;
+        _unused_ _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *rr_ref = dns_resource_record_ref(rr);
-        DnsResourceRecord *rr;
+        DnsAnswerItem *item;
-        size_t i;
+        bool found = false;
         int r;
 
         assert(a);
-        assert(rm);
+        assert(rr);
 
         /* Remove all entries matching the specified RR from *a */
 
-        DNS_ANSWER_FOREACH(rr, *a) {
+        DNS_ANSWER_FOREACH_ITEM(item, *a) {
-                r = dns_resource_record_equal(rr, rm);
+                r = dns_resource_record_equal(item->rr, rr);
                 if (r < 0)
                         return r;
-                if (r > 0)
+                if (r > 0) {
+                        dns_answer_item_unref(ordered_set_remove((*a)->items, item));
                         found = true;
-                else
+                }
-                        other = true;
-
-                if (found && other)
-                        break;
         }
 
         if (!found)
                 return 0;
 
-        if (!other) {
+        if (dns_answer_isempty(*a))
                 *a = dns_answer_unref(*a); /* Return NULL for the empty answer */
-                return 1;
-        }
-
-        if ((*a)->n_ref > 1) {
-                _cleanup_(dns_answer_unrefp) DnsAnswer *copy = NULL;
-                DnsAnswerItem *item;
-
-                copy = dns_answer_new((*a)->n_rrs);
-                if (!copy)
-                        return -ENOMEM;
-
-                DNS_ANSWER_FOREACH_ITEM(item, *a) {
-                        r = dns_resource_record_equal(item->rr, rm);
-                        if (r < 0)
-                                return r;
-                        if (r > 0)
-                                continue;
-
-                        r = dns_answer_add_raw(copy, item->rr, item->ifindex, item->flags, item->rrsig);
-                        if (r < 0)
-                                return r;
-                }
-
-                dns_answer_unref(*a);
-                *a = TAKE_PTR(copy);
-
-                return 1;
-        }
-
-        /* Only a single reference, edit in-place */
-
-        i = 0;
-        for (;;) {
-                if (i >= (*a)->n_rrs)
-                        break;
-
-                r = dns_resource_record_equal((*a)->items[i].rr, rm);
-                if (r < 0)
-                        return r;
-                if (r > 0) {
-                        /* Kill this entry */
-
-                        dns_resource_record_unref((*a)->items[i].rr);
-                        dns_resource_record_unref((*a)->items[i].rrsig);
-                        memmove((*a)->items + i, (*a)->items + i + 1, sizeof(DnsAnswerItem) * ((*a)->n_rrs - i - 1));
-                        (*a)->n_rrs--;
-                        continue;
-
-                } else
-                        /* Keep this entry */
-                        i++;
-        }
 
         return 1;
 }
@@ -655,6 +573,8 @@ int dns_answer_remove_by_answer_keys(DnsAnswer **a, DnsAnswer *b) {
                 r = dns_answer_remove_by_key(a, item->rr->key);
                 if (r < 0)
                         return r;
+                if (!*a)
+                        return 0; /* a is already empty. */
 
                 /* Let's remember this entry's RR key, to optimize the loop a bit: if we have an RRset with
                  * more than one item then we don't need to remove the key multiple times */
@@ -688,12 +608,7 @@ int dns_answer_copy_by_key(
                 if (r == 0)
                         continue;
 
-                /* Make space for at least one entry */
+                r = dns_answer_add_extend(a, item->rr, item->ifindex, item->flags|or_flags, rrsig ?: item->rrsig);
-                r = dns_answer_reserve_or_clone(a, 1);
-                if (r < 0)
-                        return r;
-
-                r = dns_answer_add(*a, item->rr, item->ifindex, item->flags|or_flags, rrsig ?: item->rrsig);
                 if (r < 0)
                         return r;
         }
@@ -707,6 +622,7 @@ int dns_answer_move_by_key(
                 const DnsResourceKey *key,
                 DnsAnswerFlags or_flags,
                 DnsResourceRecord *rrsig) {
+
         int r;
 
         assert(to);
@@ -721,140 +637,91 @@ int dns_answer_move_by_key(
 }
 
 void dns_answer_order_by_scope(DnsAnswer *a, bool prefer_link_local) {
-        DnsAnswerItem *items;
+        _cleanup_free_ DnsAnswerItem **items = NULL;
-        size_t i, start, end;
+        DnsAnswerItem **p, *item;
+        size_t n;
 
-        if (!a)
+        n = dns_answer_size(a);
+        if (n <= 1)
                 return;
 
-        if (a->n_rrs <= 1)
-                return;
-
-        start = 0;
-        end = a->n_rrs-1;
-
         /* RFC 4795, Section 2.6 suggests we should order entries
          * depending on whether the sender is a link-local address. */
 
-        items = newa(DnsAnswerItem, a->n_rrs);
+        p = items = new(DnsAnswerItem*, n);
-        for (i = 0; i < a->n_rrs; i++) {
+        if (!items)
-                if (dns_resource_record_is_link_local_address(a->items[i].rr) != prefer_link_local)
+                return (void) log_oom();
-                        /* Order address records that are not preferred to the end of the array */
-                        items[end--] = a->items[i];
-                else
-                        /* Order all other records to the beginning of the array */
-                        items[start++] = a->items[i];
-        }
 
-        assert(start == end+1);
+        /* Order preferred address records and other records to the beginning of the array */
-        memcpy(a->items, items, sizeof(DnsAnswerItem) * a->n_rrs);
+        DNS_ANSWER_FOREACH_ITEM(item, a)
+                if (dns_resource_record_is_link_local_address(item->rr) == prefer_link_local)
+                        *p++ = dns_answer_item_ref(item);
+
+        /* Order address records that are not preferred to the end of the array */
+        DNS_ANSWER_FOREACH_ITEM(item, a)
+                if (dns_resource_record_is_link_local_address(item->rr) != prefer_link_local)
+                        *p++ = dns_answer_item_ref(item);
+
+
+        assert((size_t) (p - items) == n);
+
+        ordered_set_clear(a->items);
+        for (size_t i = 0; i < n; i++)
+                assert_se(ordered_set_put(a->items, items[i]) >= 0);
 }
 
 int dns_answer_reserve(DnsAnswer **a, size_t n_free) {
-        DnsAnswer *n;
-
         assert(a);
 
         if (n_free <= 0)
                 return 0;
 
-        if (*a) {
+        if (!*a) {
-                size_t ns;
+                DnsAnswer *n;
-                int r;
 
-                if ((*a)->n_ref > 1)
-                        return -EBUSY;
-
-                ns = (*a)->n_rrs;
-                assert(ns <= UINT16_MAX); /* Maximum number of RRs we can stick into a DNS packet section */
-
-                if (n_free > UINT16_MAX - ns) /* overflow check */
-                        ns = UINT16_MAX;
-                else
-                        ns += n_free;
-
-                if ((*a)->n_allocated >= ns)
-                        return 0;
-
-                /* Allocate more than we need, but not more than UINT16_MAX */
-                if (ns <= UINT16_MAX/2)
-                        ns *= 2;
-                else
-                        ns = UINT16_MAX;
-
-                /* This must be done before realloc() below. Otherwise, the original DnsAnswer object
-                 * may be broken. */
-                r = set_reserve((*a)->set_items, ns);
-                if (r < 0)
-                        return r;
-
-                n = realloc(*a, offsetof(DnsAnswer, items) + sizeof(DnsAnswerItem) * ns);
-                if (!n)
-                        return -ENOMEM;
-
-                n->n_allocated = ns;
-
-                /* Previously all items are stored in the set, and the enough memory area is allocated
-                 * in the above. So set_put() in the below cannot fail. */
-                set_clear(n->set_items);
-                for (size_t i = 0; i < n->n_rrs; i++)
-                        assert_se(set_put(n->set_items, &n->items[i]) > 0);
-        } else {
                 n = dns_answer_new(n_free);
                 if (!n)
                         return -ENOMEM;
+
+                *a = n;
+                return 0;
         }
 
-        *a = n;
+        if ((*a)->n_ref > 1)
-        return 0;
+                return -EBUSY;
+
+        return dns_answer_reserve_internal(*a, n_free);
 }
 
 int dns_answer_reserve_or_clone(DnsAnswer **a, size_t n_free) {
+        _cleanup_(dns_answer_unrefp) DnsAnswer *n = NULL;
+        size_t ns;
         int r;
 
         assert(a);
 
-        /* Tries to extend the DnsAnswer object. And if that's not possible, since we are not the sole owner,
+        r = dns_answer_reserve(a, n_free);
-         * then allocate a new, appropriately sized one. Either way, after this call the object will only
+        if (r != -EBUSY)
-         * have a single reference, and has room for at least the specified number of RRs. */
+                return r;
 
-        if (*a && (*a)->n_ref > 1) {
+        ns = dns_answer_size(*a);
-                _cleanup_(dns_answer_unrefp) DnsAnswer *n = NULL;
+        assert(ns <= UINT16_MAX); /* Maximum number of RRs we can stick into a DNS packet section */
-                size_t ns;
 
-                ns = (*a)->n_rrs;
+        if (n_free > UINT16_MAX - ns) /* overflow check */
-                assert(ns <= UINT16_MAX); /* Maximum number of RRs we can stick into a DNS packet section */
+                ns = UINT16_MAX;
+        else
+                ns += n_free;
 
-                if (n_free > UINT16_MAX - ns) /* overflow check */
+        n = dns_answer_new(ns);
-                        ns = UINT16_MAX;
+        if (!n)
-                else if (n_free > 0) { /* Increase size and double the result, just in case — except if the
+                return -ENOMEM;
-                                        * increase is specified as 0, in which case we just allocate the
-                                        * exact amount as before, under the assumption this is just a request
-                                        * to copy the answer. */
-                        ns += n_free;
 
-                        if (ns <= UINT16_MAX/2) /* overflow check */
+        r = dns_answer_add_raw_all(n, *a);
-                                ns *= 2;
+        if (r < 0)
-                        else
+                return r;
-                                ns = UINT16_MAX;
-                }
-
-                n = dns_answer_new(ns);
-                if (!n)
-                        return -ENOMEM;
-
-                r = dns_answer_add_raw_all(n, *a);
-                if (r < 0)
-                        return r;
-
-                dns_answer_unref(*a);
-                assert_se(*a = TAKE_PTR(n));
-        } else if (n_free > 0) {
-                r = dns_answer_reserve(a, n_free);
-                if (r < 0)
-                        return r;
-        }
 
+        dns_answer_unref(*a);
+        *a = TAKE_PTR(n);
         return 0;
 }
 
@@ -944,6 +811,8 @@ int dns_answer_has_dname_for_cname(DnsAnswer *a, DnsResourceRecord *cname) {
 }
 
 void dns_answer_randomize(DnsAnswer *a) {
+        _cleanup_free_ DnsAnswerItem **items = NULL;
+        DnsAnswerItem **p, *item;
         size_t n;
 
         /* Permutes the answer list randomly (Knuth shuffle) */
@@ -952,6 +821,15 @@ void dns_answer_randomize(DnsAnswer *a) {
         if (n <= 1)
                 return;
 
+        p = items = new(DnsAnswerItem*, n);
+        if (!items)
+                return (void) log_oom();
+
+        DNS_ANSWER_FOREACH_ITEM(item, a)
+                *p++ = dns_answer_item_ref(item);
+
+        assert((size_t) (p - items) == n);
+
         for (size_t i = 0; i < n; i++) {
                 size_t k;
 
@@ -959,8 +837,12 @@ void dns_answer_randomize(DnsAnswer *a) {
                 if (k == i)
                         continue;
 
-                SWAP_TWO(a->items[i], a->items[k]);
+                SWAP_TWO(items[i], items[k]);
         }
+
+        ordered_set_clear(a->items);
+        for (size_t i = 0; i < n; i++)
+                assert_se(ordered_set_put(a->items, items[i]) >= 0);
 }
 
 uint32_t dns_answer_min_ttl(DnsAnswer *a) {

src/resolve/resolved-dns-answer.h

@@ -5,8 +5,8 @@ typedef struct DnsAnswer DnsAnswer;
 typedef struct DnsAnswerItem DnsAnswerItem;
 
 #include "macro.h"
+#include "ordered-set.h"
 #include "resolved-dns-rr.h"
-#include "set.h"
 
 /* A simple array of resource records. We keep track of the originating ifindex for each RR where that makes
  * sense, so that we can qualify A and AAAA RRs referring to a local link with the right ifindex.
@@ -29,6 +29,7 @@ typedef enum DnsAnswerFlags {
 } DnsAnswerFlags;
 
 struct DnsAnswerItem {
+        unsigned n_ref;
         DnsResourceRecord *rr;
         DnsResourceRecord *rrsig; /* Optionally, also store RRSIG RR that successfully validates this item */
         int ifindex;
@@ -37,9 +38,7 @@ struct DnsAnswerItem {
 
 struct DnsAnswer {
         unsigned n_ref;
-        Set *set_items; /* Used by dns_answer_add() for optimization. */
+        OrderedSet *items;
-        size_t n_rrs, n_allocated;
-        DnsAnswerItem items[0];
 };
 
 DnsAnswer *dns_answer_new(size_t n);
@@ -76,7 +75,7 @@ int dns_answer_move_by_key(DnsAnswer **to, DnsAnswer **from, const DnsResourceKe
 int dns_answer_has_dname_for_cname(DnsAnswer *a, DnsResourceRecord *cname);
 
 static inline size_t dns_answer_size(DnsAnswer *a) {
-        return a ? a->n_rrs : 0;
+        return a ? ordered_set_size(a->items) : 0;
 }
 
 static inline bool dns_answer_isempty(DnsAnswer *a) {
@@ -91,50 +90,40 @@ uint32_t dns_answer_min_ttl(DnsAnswer *a);
 
 DEFINE_TRIVIAL_CLEANUP_FUNC(DnsAnswer*, dns_answer_unref);
 
-#define _DNS_ANSWER_FOREACH(q, kk, a)                                   \
+typedef struct DnsAnswerIterator {
-        for (size_t UNIQ_T(i, q) = ({                                   \
+        Iterator iterator;
-                        (kk) = dns_answer_isempty(a) ? NULL : (a)->items[0].rr; \
+        DnsAnswer *answer;
-                        0;                                              \
+        DnsAnswerItem *item;
-                });                                                     \
+} DnsAnswerIterator;
-             UNIQ_T(i, q) < dns_answer_size(a);                         \
-             UNIQ_T(i, q)++,                                            \
-                     (kk) = UNIQ_T(i, q) < dns_answer_size(a) ? (a)->items[UNIQ_T(i, q)].rr : NULL)
 
-#define DNS_ANSWER_FOREACH(kk, a) _DNS_ANSWER_FOREACH(UNIQ, kk, a)
+#define _DNS_ANSWER_FOREACH(kk, a, i)                                   \
+        for (DnsAnswerIterator i = { .iterator = ITERATOR_FIRST, .answer = (a) };  \
+             i.answer &&                                                \
+             ordered_set_iterate(i.answer->items, &i.iterator, (void**) &(i.item)) && \
+             (kk = i.item->rr, true); )
 
-#define _DNS_ANSWER_FOREACH_IFINDEX(q, kk, ifi, a)                      \
+#define DNS_ANSWER_FOREACH(rr, a) _DNS_ANSWER_FOREACH(rr, a, UNIQ_T(i, UNIQ))
-        for (size_t UNIQ_T(i, q) = ({                                   \
-                                (kk) = dns_answer_isempty(a) ? NULL : (a)->items[0].rr; \
-                                (ifi) = dns_answer_isempty(a) ? 0 : (a)->items[0].ifindex; \
-                                0;                                      \
-                        });                                             \
-             UNIQ_T(i, q) < dns_answer_size(a);                         \
-             UNIQ_T(i, q)++,                                            \
-                     (kk) = UNIQ_T(i, q) < dns_answer_size(a) ? (a)->items[UNIQ_T(i, q)].rr : NULL, \
-                     (ifi) = UNIQ_T(i, q) < dns_answer_size(a) ? (a)->items[UNIQ_T(i, q)].ifindex : 0)
 
-#define DNS_ANSWER_FOREACH_IFINDEX(kk, ifindex, a) _DNS_ANSWER_FOREACH_IFINDEX(UNIQ, kk, ifindex, a)
+#define _DNS_ANSWER_FOREACH_IFINDEX(kk, ifi, a, i)                      \
+        for (DnsAnswerIterator i = { .iterator = ITERATOR_FIRST, .answer = (a) };  \
+             i.answer &&                                                \
+             ordered_set_iterate(i.answer->items, &i.iterator, (void**) &(i.item)) && \
+             (kk = i.item->rr, ifi = i.item->ifindex, true); )
 
-#define _DNS_ANSWER_FOREACH_FLAGS(q, kk, fl, a)                         \
+#define DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, a) _DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, a, UNIQ_T(i, UNIQ))
-        for (size_t UNIQ_T(i, q) = ({                                   \
-                                (kk) = dns_answer_isempty(a) ? NULL : (a)->items[0].rr; \
-                                (fl) = dns_answer_isempty(a) ? 0 : (a)->items[0].flags; \
-                                0;                                      \
-                        });                                             \
-             UNIQ_T(i, q) < dns_answer_size(a);                         \
-             UNIQ_T(i, q)++,                                            \
-                     (kk) = UNIQ_T(i, q) < dns_answer_size(a) ? (a)->items[UNIQ_T(i, q)].rr : NULL, \
-                     (fl) = UNIQ_T(i, q) < dns_answer_size(a) ? (a)->items[UNIQ_T(i, q)].flags : 0)
 
-#define DNS_ANSWER_FOREACH_FLAGS(kk, flags, a) _DNS_ANSWER_FOREACH_FLAGS(UNIQ, kk, flags, a)
+#define _DNS_ANSWER_FOREACH_FLAGS(kk, fl, a, i)                         \
+        for (DnsAnswerIterator i = { .iterator = ITERATOR_FIRST, .answer = (a) };  \
+             i.answer &&                                                \
+             ordered_set_iterate(i.answer->items, &i.iterator, (void**) &(i.item)) && \
+             (kk = i.item->rr, fl = i.item->flags, true); )
 
-#define _DNS_ANSWER_FOREACH_ITEM(q, item, a)                            \
+#define DNS_ANSWER_FOREACH_FLAGS(rr, flags, a) _DNS_ANSWER_FOREACH_FLAGS(rr, flags, a, UNIQ_T(i, UNIQ))
-        for (size_t UNIQ_T(i, q) = ({                                   \
-                                (item) = dns_answer_isempty(a) ? NULL : (a)->items; \
-                                0;                                      \
-                        });                                             \
-             UNIQ_T(i, q) < dns_answer_size(a);                         \
-             UNIQ_T(i, q)++,                                            \
-                     (item) = (UNIQ_T(i, q) < dns_answer_size(a)) ? (a)->items + UNIQ_T(i, q) : NULL)
 
-#define DNS_ANSWER_FOREACH_ITEM(item, a) _DNS_ANSWER_FOREACH_ITEM(UNIQ, item, a)
+#define _DNS_ANSWER_FOREACH_ITEM(item, a, i)                            \
+        for (DnsAnswerIterator i = { .iterator = ITERATOR_FIRST, .answer = (a) };  \
+             i.answer &&                                                \
+             ordered_set_iterate(i.answer->items, &i.iterator, (void**) &(i.item)) && \
+             (item = i.item, true); )
+
+#define DNS_ANSWER_FOREACH_ITEM(item, a) _DNS_ANSWER_FOREACH_ITEM(item, a, UNIQ_T(i, UNIQ))

src/resolve/resolved-dns-dnssec.c

@@ -989,8 +989,9 @@ int dnssec_verify_rrset(
 
         DnsResourceRecord **list, *rr;
         const char *source, *name;
-        size_t n = 0, sig_size;
         _cleanup_free_ char *sig_data = NULL;
+        size_t sig_size = 0; /* avoid false maybe-uninitialized warning */
+        size_t n = 0;
         bool wildcard;
         int r;
 
@@ -1184,7 +1185,7 @@ int dnssec_verify_rrset_search(
 
         /* Verifies all RRs from "a" that match the key "key" against DNSKEYs in "validated_dnskeys" */
 
-        if (!a || a->n_rrs <= 0)
+        if (dns_answer_isempty(a))
                 return -ENODATA;
 
         /* Iterate through each RRSIG RR. */

src/resolve/resolved-dns-transaction.c

@@ -3106,6 +3106,7 @@ static int dnssec_validate_records(
         /* Returns negative on error, 0 if validation failed, 1 to restart validation, 2 when finished. */
 
         DNS_ANSWER_FOREACH(rr, t->answer) {
+                _unused_ _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *rr_ref = dns_resource_record_ref(rr);
                 DnsResourceRecord *rrsig = NULL;
                 DnssecResult result;
 

src/resolve/resolved-dns-trust-anchor.c

@@ -599,6 +599,7 @@ static int dns_trust_anchor_revoked_put(DnsTrustAnchor *d, DnsResourceRecord *rr
 static int dns_trust_anchor_remove_revoked(DnsTrustAnchor *d, DnsResourceRecord *rr) {
         _cleanup_(dns_answer_unrefp) DnsAnswer *new_answer = NULL;
         DnsAnswer *old_answer;
+        DnsAnswerItem *item;
         int r;
 
         /* Remember that this is a revoked trust anchor RR */
@@ -631,11 +632,12 @@ static int dns_trust_anchor_remove_revoked(DnsTrustAnchor *d, DnsResourceRecord
                 return 1;
         }
 
-        r = hashmap_replace(d->positive_by_key, new_answer->items[0].rr->key, new_answer);
+        item = ordered_set_first(new_answer->items);
+        r = hashmap_replace(d->positive_by_key, item->rr->key, new_answer);
         if (r < 0)
                 return r;
 
-        new_answer = NULL;
+        TAKE_PTR(new_answer);
         dns_answer_unref(old_answer);
         return 1;
 }

src/resolve/resolved-mdns.c

@@ -107,7 +107,8 @@ static int proposed_rrs_cmp(DnsResourceRecord **x, unsigned x_size, DnsResourceR
 
 static int mdns_packet_extract_matching_rrs(DnsPacket *p, DnsResourceKey *key, DnsResourceRecord ***ret_rrs) {
         _cleanup_free_ DnsResourceRecord **list = NULL;
-        unsigned n = 0, size = 0;
+        size_t i, n = 0, size = 0;
+        DnsResourceRecord *rr;
         int r;
 
         assert(p);
@@ -115,28 +116,39 @@ static int mdns_packet_extract_matching_rrs(DnsPacket *p, DnsResourceKey *key, D
         assert(ret_rrs);
         assert_return(DNS_PACKET_NSCOUNT(p) > 0, -EINVAL);
 
-        for (size_t i = DNS_PACKET_ANCOUNT(p); i < (DNS_PACKET_ANCOUNT(p) + DNS_PACKET_NSCOUNT(p)); i++) {
+        i = 0;
-                r = dns_resource_key_match_rr(key, p->answer->items[i].rr, NULL);
+        DNS_ANSWER_FOREACH(rr, p->answer) {
-                if (r < 0)
+                if (i >= DNS_PACKET_ANCOUNT(p) && i < DNS_PACKET_ANCOUNT(p) + DNS_PACKET_NSCOUNT(p)) {
-                        return r;
+                        r = dns_resource_key_match_rr(key, rr, NULL);
-                if (r > 0)
+                        if (r < 0)
-                        size++;
+                                return r;
+                        if (r > 0)
+                                size++;
+                }
+                i++;
         }
 
-        if (size == 0)
+        if (size == 0) {
+                *ret_rrs = NULL;
                 return 0;
+        }
 
         list = new(DnsResourceRecord *, size);
         if (!list)
                 return -ENOMEM;
 
-        for (size_t i = DNS_PACKET_ANCOUNT(p); i < (DNS_PACKET_ANCOUNT(p) + DNS_PACKET_NSCOUNT(p)); i++) {
+        i = 0;
-                r = dns_resource_key_match_rr(key, p->answer->items[i].rr, NULL);
+        DNS_ANSWER_FOREACH(rr, p->answer) {
-                if (r < 0)
+                if (i >= DNS_PACKET_ANCOUNT(p) && i < DNS_PACKET_ANCOUNT(p) + DNS_PACKET_NSCOUNT(p)) {
-                        return r;
+                        r = dns_resource_key_match_rr(key, rr, NULL);
-                if (r > 0)
+                        if (r < 0)
-                        list[n++] = p->answer->items[i].rr;
+                                return r;
+                        if (r > 0)
+                                list[n++] = rr;
+                }
+                i++;
         }
+
         assert(n == size);
         typesafe_qsort(list, size, mdns_rr_compare);