Make iterators less fragile.

Instead of storing a pointer to the argdata_t, which stores the pointers
to the underlying data, store the pointers to the underlying data in the
iterator directly. This way, the iterator still works when the argdata_t
is gone, but the data still exists.

src/argdata_impl.h

@@ -41,8 +41,12 @@ struct argdata_t {
 
 struct argdata_map_iterator_impl {
   alignas(long) int error;
-  const argdata_t *container;
-  size_t offset;
+  union {
+    const uint8_t *buf;
+    const argdata_t *const *keys;
+  };
+  const argdata_t *const *values; // When NULL, we're iterating a buffer.
+  size_t len;
   argdata_t key;
   argdata_t value;
 };
@@ -59,8 +63,9 @@ static_assert(offsetof(struct argdata_map_iterator_impl, error) ==
 
 struct argdata_seq_iterator_impl {
   alignas(long) int error;
-  const argdata_t *container;
-  size_t offset;
+  const uint8_t *buf;
+  const argdata_t *const *entries; // When NULL, we're iterating a buffer.
+  size_t len;
   argdata_t value;
 };
 

src/argdata_map_iterate.c

@@ -12,29 +12,26 @@ int argdata_map_iterate(const argdata_t *ad, argdata_map_iterator_t *it_) {
   struct argdata_map_iterator_impl *it =
       (struct argdata_map_iterator_impl *)it_;
   switch (ad->type) {
-    case AD_BUFFER: {
-      const uint8_t *buf = ad->buffer;
-      size_t len = ad->length;
-      it->container = ad;
-      it->error = parse_type(ADT_MAP, &buf, &len);
-      it->offset = buf - ad->buffer;
+    case AD_BUFFER:
+      it->buf = ad->buffer;
+      it->len = ad->length;
+      it->error = parse_type(ADT_MAP, &it->buf, &it->len);
+      it->values = NULL;
       break;
-    }
     case AD_MAP:
-      it->container = ad;
+      it->keys = ad->map.keys;
+      it->values = ad->map.values;
+      it->len = ad->map.count;
       it->error = 0;
-      it->offset = 0;
       break;
     default:
       it->error = EINVAL;
       break;
   }
   if (it->error != 0) {
-    // If the iterator is invalid, fall back to using an empty buffer,
-    // so that calls to argdata_map_next() act as if iterating an empty
-    // mapping.
-    it->container = &argdata_null;
-    it->offset = 0;
+    // If the iterator is invalid, set len to zero so that calls to
+    // argdata_map_next() act as if iterating an empty mapping.
+    it->len = 0;
   }
   return it->error;
 }

src/argdata_map_next.c

@@ -13,32 +13,26 @@ bool argdata_map_next(argdata_map_iterator_t *it_, const argdata_t **key,
                       const argdata_t **value) {
   struct argdata_map_iterator_impl *it =
       (struct argdata_map_iterator_impl *)it_;
-  const argdata_t *ad = it->container;
-  if (ad->type == AD_BUFFER) {
-    const uint8_t *buf = ad->buffer + it->offset;
-    size_t len = ad->length - it->offset;
-    if (len == 0)
-      return false;
-    int error = parse_subfield(&it->key, &buf, &len);
+  if (it->len == 0)
+    return false;
+  if (it->values == NULL) {
+    int error = parse_subfield(&it->key, &it->buf, &it->len);
     if (error != 0) {
       it->error = error;
       return false;
     }
-    error = parse_subfield(&it->value, &buf, &len);
+    error = parse_subfield(&it->value, &it->buf, &it->len);
     if (error != 0) {
       it->error = error;
       return false;
     }
-    it->offset = buf - ad->buffer;
     *key = &it->key;
     *value = &it->value;
     return true;
   } else {
-    assert(ad->type == AD_MAP && "Iterator points to value of the wrong type");
-    if (it->offset >= ad->map.count)
-      return false;
-    *key = ad->map.keys[it->offset];
-    *value = ad->map.values[it->offset++];
+    *key = *it->keys++;
+    *value = *it->values++;
+    --it->len;
     return true;
   }
 }

src/argdata_seq_iterate.c

@@ -12,29 +12,25 @@ int argdata_seq_iterate(const argdata_t *ad, argdata_seq_iterator_t *it_) {
   struct argdata_seq_iterator_impl *it =
       (struct argdata_seq_iterator_impl *)it_;
   switch (ad->type) {
-    case AD_BUFFER: {
-      const uint8_t *buf = ad->buffer;
-      size_t len = ad->length;
-      it->container = ad;
-      it->error = parse_type(ADT_SEQ, &buf, &len);
-      it->offset = buf - ad->buffer;
+    case AD_BUFFER:
+      it->buf = ad->buffer;
+      it->len = ad->length;
+      it->error = parse_type(ADT_SEQ, &it->buf, &it->len);
+      it->entries = NULL;
       break;
-    }
     case AD_SEQ:
-      it->container = ad;
+      it->entries = ad->seq.entries;
+      it->len = ad->seq.count;
       it->error = 0;
-      it->offset = 0;
       break;
     default:
       it->error = EINVAL;
       break;
   }
   if (it->error != 0) {
-    // If the iterator is invalid, fall back to using an empty buffer,
-    // so that calls to argdata_seq_next() act as if iterating an empty
-    // sequence.
-    it->container = &argdata_null;
-    it->offset = 0;
+    // If the iterator is invalid, set len to zero so that calls to
+    // argdata_seq_next() act as if iterating an empty sequence.
+    it->len = 0;
   }
   return it->error;
 }

src/argdata_seq_next.c

@@ -12,25 +12,19 @@
 bool argdata_seq_next(argdata_seq_iterator_t *it_, const argdata_t **value) {
   struct argdata_seq_iterator_impl *it =
       (struct argdata_seq_iterator_impl *)it_;
-  const argdata_t *ad = it->container;
-  if (ad->type == AD_BUFFER) {
-    const uint8_t *buf = ad->buffer + it->offset;
-    size_t len = ad->length - it->offset;
-    if (len == 0)
-      return false;
-    int error = parse_subfield(&it->value, &buf, &len);
+  if (it->len == 0)
+    return false;
+  if (it->entries == NULL) {
+    int error = parse_subfield(&it->value, &it->buf, &it->len);
     if (error != 0) {
       it->error = error;
       return false;
     }
-    it->offset = buf - ad->buffer;
     *value = &it->value;
     return true;
   } else {
-    assert(ad->type == AD_SEQ && "Iterator points to value of the wrong type");
-    if (it->offset >= ad->seq.count)
-      return false;
-    *value = ad->seq.entries[it->offset++];
+    *value = *it->entries++;
+    --it->len;
     return true;
   }
 }