From: Rusty Russell <rusty@rustcorp.com.au>
Date: Wed, 7 Sep 2016 04:27:01 +0000 (+0930)
Subject: tal: make tal_next() only return immediate children.
X-Git-Url: http://git.ozlabs.org/?p=ccan;a=commitdiff_plain;h=e81b527384ad5331a9cd35a9f21bc8ac6b16d137

tal: make tal_next() only return immediate children.

I tried to use it and got this wrong: moreover, I wanted to control
topology, which requires nested iteration, and skip children
of a node which I knew was changing.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---

diff --git a/ccan/tal/path/test/run-basename.c b/ccan/tal/path/test/run-basename.c
index c99353a4..23188481 100644
--- a/ccan/tal/path/test/run-basename.c
+++ b/ccan/tal/path/test/run-basename.c
@@ -47,7 +47,7 @@ int main(void)
 	path = path_basename(ctx, take(tal_strdup(ctx, "..")));
 	ok1(streq(path, ".."));
 	ok1(tal_parent(path) == ctx);
-	ok1(tal_first(ctx) == path && !tal_next(ctx, path));
+	ok1(tal_first(ctx) == path && !tal_next(path));
 	tal_free(path);
 	ok1(path_basename(ctx, take(NULL)) == NULL);
 	ok1(!tal_first(ctx));
diff --git a/ccan/tal/path/test/run-canon.c b/ccan/tal/path/test/run-canon.c
index e01567bf..c6e3a8b0 100644
--- a/ccan/tal/path/test/run-canon.c
+++ b/ccan/tal/path/test/run-canon.c
@@ -36,7 +36,7 @@ int main(void)
 	ok1(strstarts(path, cwd));
 	ok1(path[strlen(cwd)] == PATH_SEP);
 	ok1(strlen(path) == strlen(cwd) + 1 + strlen("run-canon-foo"));
-	ok1(tal_first(ctx) == path && tal_next(ctx, path) == NULL);
+	ok1(tal_first(ctx) == path && tal_next(path) == NULL);
 	path2 = path_canon(ctx, "run-canon-link");
 	ok1(streq(path2, path));
 
diff --git a/ccan/tal/path/test/run-dirname.c b/ccan/tal/path/test/run-dirname.c
index 46589aef..a0bc2cbe 100644
--- a/ccan/tal/path/test/run-dirname.c
+++ b/ccan/tal/path/test/run-dirname.c
@@ -47,7 +47,7 @@ int main(void)
 	path = path_dirname(ctx, take(tal_strdup(ctx, "..")));
 	ok1(streq(path, "."));
 	ok1(tal_parent(path) == ctx);
-	ok1(tal_first(ctx) == path && !tal_next(ctx, path));
+	ok1(tal_first(ctx) == path && !tal_next(path));
 	tal_free(path);
 	ok1(path_dirname(ctx, take(NULL)) == NULL);
 	ok1(!tal_first(ctx));
diff --git a/ccan/tal/path/test/run-join.c b/ccan/tal/path/test/run-join.c
index a4f63b31..3961cbb1 100644
--- a/ccan/tal/path/test/run-join.c
+++ b/ccan/tal/path/test/run-join.c
@@ -32,39 +32,39 @@ int main(void)
 	path = path_join(ctx, "foo", take(tal_strdup(ctx, "bar")));
 	ok1(streq(path, "foo/bar"));
 	ok1(tal_parent(path) == ctx);
-	ok1(tal_first(ctx) == path && tal_next(ctx, path) == NULL);
+	ok1(tal_first(ctx) == path && tal_next(path) == NULL && tal_first(path) == NULL);
 	tal_free(path);
 
 	path = path_join(ctx, "foo", take(tal_strdup(ctx, "/bar")));
 	ok1(streq(path, "/bar"));
 	ok1(tal_parent(path) == ctx);
-	ok1(tal_first(ctx) == path && tal_next(ctx, path) == NULL);
+	ok1(tal_first(ctx) == path && tal_next(path) == NULL && tal_first(path) == NULL);
 	tal_free(path);
 
 	path = path_join(ctx, take(tal_strdup(ctx, "foo")), "bar");
 	ok1(streq(path, "foo/bar"));
 	ok1(tal_parent(path) == ctx);
-	ok1(tal_first(ctx) == path && tal_next(ctx, path) == NULL);
+	ok1(tal_first(ctx) == path && tal_next(path) == NULL && tal_first(path) == NULL);
 	tal_free(path);
 
 	path = path_join(ctx, take(tal_strdup(ctx, "foo")), "/bar");
 	ok1(streq(path, "/bar"));
 	ok1(tal_parent(path) == ctx);
-	ok1(tal_first(ctx) == path && tal_next(ctx, path) == NULL);
+	ok1(tal_first(ctx) == path && tal_next(path) == NULL && tal_first(path) == NULL);
 	tal_free(path);
 
 	path = path_join(ctx, take(tal_strdup(ctx, "foo")),
 			 take(tal_strdup(ctx, "bar")));
 	ok1(streq(path, "foo/bar"));
 	ok1(tal_parent(path) == ctx);
-	ok1(tal_first(ctx) == path && tal_next(ctx, path) == NULL);
+	ok1(tal_first(ctx) == path && tal_next(path) == NULL && tal_first(path) == NULL);
 	tal_free(path);
 
 	path = path_join(ctx, take(tal_strdup(ctx, "foo")),
 			 take(tal_strdup(ctx, "/bar")));
 	ok1(streq(path, "/bar"));
 	ok1(tal_parent(path) == ctx);
-	ok1(tal_first(ctx) == path && tal_next(ctx, path) == NULL);
+	ok1(tal_first(ctx) == path && tal_next(path) == NULL && tal_first(path) == NULL);
 	tal_free(path);
 
 	path = path_join(ctx, take(NULL), "bar");
diff --git a/ccan/tal/path/test/run-readlink.c b/ccan/tal/path/test/run-readlink.c
index 28dcf87e..bab04170 100644
--- a/ccan/tal/path/test/run-readlink.c
+++ b/ccan/tal/path/test/run-readlink.c
@@ -30,7 +30,7 @@ int main(void)
 	link = path_readlink(ctx, take(tal_strdup(ctx, "run-readlink-link")));
 	ok1(tal_parent(link) == ctx);
 	ok1(streq(link, "/tmp"));
-	ok1(tal_first(ctx) == link && tal_next(ctx, link) == NULL);
+	ok1(tal_first(ctx) == link && tal_next(link) == NULL && tal_first(link) == NULL);
 
 	unlink("run-readlink-link");
 
diff --git a/ccan/tal/path/test/run-split.c b/ccan/tal/path/test/run-split.c
index 732333c3..1083cc88 100644
--- a/ccan/tal/path/test/run-split.c
+++ b/ccan/tal/path/test/run-split.c
@@ -98,7 +98,7 @@ int main(void)
 	ok1(!split);
 	ok1(tal_first(ctx) == NULL);
 
-	ok1(tal_first(NULL) == ctx && tal_next(NULL, ctx) == NULL);
+	ok1(tal_first(NULL) == ctx && tal_next(ctx) == NULL && tal_first(ctx) == NULL);
 	tal_free(ctx);
 
 	return exit_status();
diff --git a/ccan/tal/str/test/helper.h b/ccan/tal/str/test/helper.h
index c1bc9ccf..5f0b68f5 100644
--- a/ccan/tal/str/test/helper.h
+++ b/ccan/tal/str/test/helper.h
@@ -17,6 +17,6 @@ static inline bool no_children(const void *ctx)
 
 static inline bool single_child(const void *ctx, const void *child)
 {
-	return tal_first(ctx) == child && !tal_next(ctx, child);
+	return tal_first(ctx) == child && !tal_next(child) && !tal_first(child);
 }
 #endif
diff --git a/ccan/tal/tal.c b/ccan/tal/tal.c
index 1eaa5749..8360b252 100644
--- a/ccan/tal/tal.c
+++ b/ccan/tal/tal.c
@@ -630,31 +630,16 @@ tal_t *tal_first(const tal_t *root)
 	return from_tal_hdr(c);
 }
 
-tal_t *tal_next(const tal_t *root, const tal_t *prev)
+tal_t *tal_next(const tal_t *prev)
 {
-        struct tal_hdr *c, *t = debug_tal(to_tal_hdr(prev)), *top;
+        struct tal_hdr *next, *prevhdr = debug_tal(to_tal_hdr(prev));
+	struct list_head *head;
 
-        /* Children? */
-	c = first_child(t);
-	if (c)
-		return from_tal_hdr(c);
-
-        top = to_tal_hdr_or_null(root);
-        do {
-		struct tal_hdr *next;
-		struct list_node *end;
-
-		end = &ignore_destroying_bit(t->parent_child)->children.n;
-
-		next = list_entry(t->list.next, struct tal_hdr, list);
-		if (&next->list != end)
-			return from_tal_hdr(next);
-
-                /* OK, go back to parent. */
-                t = ignore_destroying_bit(t->parent_child)->parent;
-        } while (t != top);
-
-        return NULL;
+	head = &ignore_destroying_bit(prevhdr->parent_child)->children;
+	next = list_next(head, prevhdr, list);
+	if (!next)
+		return NULL;
+	return from_tal_hdr(next);
 }
 
 tal_t *tal_parent(const tal_t *ctx)
diff --git a/ccan/tal/tal.h b/ccan/tal/tal.h
index f360a961..548b9274 100644
--- a/ccan/tal/tal.h
+++ b/ccan/tal/tal.h
@@ -262,7 +262,7 @@ const char *tal_name(const tal_t *ptr);
 size_t tal_count(const tal_t *ptr);
 
 /**
- * tal_first - get the first tal object child.
+ * tal_first - get the first immediate tal object child.
  * @root: The tal allocated object to start with, or NULL.
  *
  * Returns NULL if there are no children.
@@ -270,15 +270,13 @@ size_t tal_count(const tal_t *ptr);
 tal_t *tal_first(const tal_t *root);
 
 /**
- * tal_next - get the next tal object child.
- * @root: The tal allocated object to start with, or NULL.
+ * tal_next - get the next immediate tal object child.
  * @prev: The return value from tal_first or tal_next.
  *
- * Returns NULL if there are no more children.  This should be safe to
- * call on an altering tree unless @prev is no longer a descendent of
- * @root.
+ * Returns NULL if there are no more immediate children.  This should be safe to
+ * call on an altering tree unless @prev is no longer valid.
  */
-tal_t *tal_next(const tal_t *root, const tal_t *prev);
+tal_t *tal_next(const const tal_t *prev);
 
 /**
  * tal_parent - get the parent of a tal object.
diff --git a/ccan/tal/test/run-expand.c b/ccan/tal/test/run-expand.c
index 607947be..841735fb 100644
--- a/ccan/tal/test/run-expand.c
+++ b/ccan/tal/test/run-expand.c
@@ -24,7 +24,7 @@ int main(void)
 	ok1(a[1] == 1);
 	ok1(a[2] == 2);
 	ok1(a[3] == 0);
-	ok1(tal_first(NULL) == a && !tal_next(NULL, a));
+	ok1(tal_first(NULL) == a && !tal_next(a) && !tal_first(a));
 
 	tal_free(a);
 
diff --git a/ccan/tal/test/run-iter.c b/ccan/tal/test/run-iter.c
index 561e09cc..5992172a 100644
--- a/ccan/tal/test/run-iter.c
+++ b/ccan/tal/test/run-iter.c
@@ -4,9 +4,33 @@
 
 #define NUM 1000
 
+static int set_children(const tal_t *parent, char val)
+{
+	char *iter;
+	int num = 0;
+
+	for (iter = tal_first(parent); iter; iter = tal_next(iter)) {
+		ok1(*iter == '0');
+		*iter = val;
+		num++;
+		num += set_children(iter, val);
+	}
+	return num;
+}
+
+static void check_children(const tal_t *parent, char val)
+{
+	const char *iter;
+
+	for (iter = tal_first(parent); iter; iter = tal_next(iter)) {
+		ok1(*iter == val);
+		check_children(iter, val);
+	}
+}
+
 int main(void)
 {
-	char *p[NUM] = { NULL }, *iter;
+	char *p[NUM] = { NULL };
 	int i;
 
 	plan_tests(NUM + 1 + NUM);
@@ -17,18 +41,13 @@ int main(void)
 		*p[i] = '0';
 	}
 
-	i = 0;
-	for (iter = tal_first(NULL); iter; iter = tal_next(NULL, iter)) {
-		i++;
-		ok1(*iter == '0');
-		*iter = '1';
-	}
+	i = set_children(NULL, '1');
 	ok1(i == NUM);
 
-	for (i = NUM-1; i >= 0; i--) {
-		ok1(*p[i] == '1');
+	check_children(NULL, '1');
+	for (i = NUM-1; i >= 0; i--)
 		tal_free(p[i]);
-	}
+
 	tal_cleanup();
 	return exit_status();
 }
diff --git a/ccan/tal/test/run-overflow.c b/ccan/tal/test/run-overflow.c
index e68c0471..d975398b 100644
--- a/ccan/tal/test/run-overflow.c
+++ b/ccan/tal/test/run-overflow.c
@@ -48,7 +48,7 @@ int main(void)
 		     (size_t)-1UL / sizeof(int) / 2);
 	ok1(!pi);
 	ok1(error_count == 4);
-	ok1(tal_first(NULL) == origpi && !tal_next(NULL, origpi));
+	ok1(tal_first(NULL) == origpi && !tal_next(origpi) && !tal_first(origpi));
 	tal_free(origpi);
 
 	/* Now, check that with taltk() we free old one on failure. */
diff --git a/ccan/tal/test/run.c b/ccan/tal/test/run.c
index 98486512..4931099e 100644
--- a/ccan/tal/test/run.c
+++ b/ccan/tal/test/run.c
@@ -25,7 +25,7 @@ int main(void)
 
 	/* Iteration test. */
 	i = 0;
-	for (p = tal_first(parent); p; p = tal_next(parent, p)) {
+	for (p = tal_first(parent); p; p = tal_next(p)) {
 		*p = '1';
 		i++;
 	}