On Tue, 2010-10-26 at 16:43 -0400, Francis Giraldeau wrote: > Le mardi 26 octobre 2010 à 11:21 -0700, David Lutterkort a écrit : > > Hi Francis, > > > > On Fri, 2010-10-22 at 14:09 -0400, [email protected] > > wrote: > > > diff --git a/src/get.c b/src/get.c > > > index 11ea5de..9d420af 100644 > > > --- a/src/get.c > > > +++ b/src/get.c > > > @@ -572,6 +572,7 @@ static struct tree *get_quant_star(struct lens *lens, > > > struct state *state) { > > > > > > t = get_lens(lens->child, state); > > > list_tail_cons(tree, tail, t); > > > + list_update_tail(tree, tail); > > > > Why is a new macro needed here ? The list_tail_cons macro should already > > maintain tail as the tail of the list tree. If it doesn't, there's a bug > > in it. > > It has a bug only on the first call, when tail is NULL. If tree has more > than one node, the tail is not updated appropriately.
Ugh .. I am amazed things worked this long with such a fundamental bug. I shuffled your patch around a little - I wanted to make it clearer in list_tail_cons that the operation consists of two steps (append to list, forward tail pointer) I also found one case where this actually lead to an incorrect tree in the tests (in test_interfaces.aug). If you are ok with the attached patch, feel free to push it. David
>From d705dea453e1ff448dacca34724556d77ccac979 Mon Sep 17 00:00:00 2001 From: Francis Giraldeau <[email protected]> Date: Fri, 22 Oct 2010 13:56:03 -0400 Subject: [PATCH] * src/list.h (list_tail_cons): behave properly when ELT is a list itself Fixes bug #144: wrong behavior of iter concat This patch fix the list_tail_cons macro to fastforward the tail list when the first item is added to the list, otherwise if the first item added when the list is NULL has two item, the tail points the the first item. --- lenses/tests/test_interfaces.aug | 2 +- src/list.h | 10 +++++++--- tests/modules/pass_iter_concat.aug | 7 +++++++ 3 files changed, 15 insertions(+), 4 deletions(-) create mode 100644 tests/modules/pass_iter_concat.aug diff --git a/lenses/tests/test_interfaces.aug b/lenses/tests/test_interfaces.aug index 0c40296..1bd19c5 100644 --- a/lenses/tests/test_interfaces.aug +++ b/lenses/tests/test_interfaces.aug @@ -47,6 +47,7 @@ mapping eth1 { "2" = "eth0" } { "3" = "#foo" } } { "allow-hotplug" { "1" = "eth1" } } + { } { "iface" = "lo" { "family" = "inet"} { "method" = "loopback"} {} } @@ -86,4 +87,3 @@ test Interfaces.lns put "" after set "/iface[1]/family" "inet"; set "/iface[1]/method" "dhcp" = "iface eth0 inet dhcp\n" - diff --git a/src/list.h b/src/list.h index eb9f84f..c757cc6 100644 --- a/src/list.h +++ b/src/list.h @@ -108,20 +108,24 @@ } while (0) /* Append ELT to the end of LIST. TAIL must be NULL or a pointer to - the last element of LIST + the last element of LIST. ELT may also be a list */ #define list_tail_cons(list, tail, elt) \ do { \ + /* Append ELT at the end of LIST */ \ if ((list) == NULL) { \ (list) = (elt); \ - (tail) = (list); \ } else { \ if ((tail) == NULL) \ for ((tail) = (list); (tail)->next != NULL; \ (tail) = (tail)->next); \ (tail)->next = (elt); \ - (tail) = (elt); \ } \ + /* Make sure TAIL is the last element on the combined LIST */ \ + (tail) = (elt); \ + if ((tail) != NULL) \ + while ((tail)->next != NULL) \ + (tail) = (tail)->next; \ } while(0) /* diff --git a/tests/modules/pass_iter_concat.aug b/tests/modules/pass_iter_concat.aug new file mode 100644 index 0000000..00dc4d8 --- /dev/null +++ b/tests/modules/pass_iter_concat.aug @@ -0,0 +1,7 @@ +module Pass_iter_concat = + +let l1 = [ key "a" . del "x" "x" ] +let l2 = [ key "b" . del "y" "y" ] +let l3 = (l1 . l2)* + +test l3 get "axbyaxby" = { "a" }{ "b" }{ "a" }{ "b" } -- 1.7.2.3
_______________________________________________ augeas-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/augeas-devel
