From ede481d4c08ca3edaf889bba9ef30bd88c2170ba Mon Sep 17 00:00:00 2001 From: Jakub Sitnicki Date: Thu, 14 Aug 2014 07:42:25 +0200 Subject: [PATCH 1/3] mu: Test splicing child containers when there is only one thread This test reproduces a regression introduced by commit 97101f1f82ba ("mu: Prune empty container when an only child gets promoted to the root set"). When the results of a mu-find query contain only a one thread: $ mu find --threads --fields 'd s' '' Sat 09 Aug 2014 07:00:00 PM CEST [mu4e] Test Message `-> Sat 09 Aug 2014 08:30:00 PM CEST Re: [mu4e] Test Message ... and we narrow down the query in such a way that the root message gets excluded, then a crash occurs: $ mu find --threads --fields 'd s' '' date:2014-08-09/20:00..2014-08-09/21:00 ** ERROR:mu-container.c:117:mu_container_append_siblings: assertion failed: (c) Aborted (core dumped) Reported-by: Josiah Schwab --- .gitignore | 1 + lib/tests/Makefile.am | 4 ++ lib/tests/test-mu-container.c | 83 +++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+) create mode 100644 lib/tests/test-mu-container.c diff --git a/.gitignore b/.gitignore index 5eddfc5f..9a6e3c10 100644 --- a/.gitignore +++ b/.gitignore @@ -54,6 +54,7 @@ msg2pdf test-mu-cmd test-mu-cmd-cfind test-mu-contacts +test-mu-container test-mu-date test-mu-flags test-mu-maildir diff --git a/lib/tests/Makefile.am b/lib/tests/Makefile.am index 8125c8fc..46ff7345 100644 --- a/lib/tests/Makefile.am +++ b/lib/tests/Makefile.am @@ -70,6 +70,10 @@ TEST_PROGS += test-mu-flags test_mu_flags_SOURCES= test-mu-flags.c dummy.cc test_mu_flags_LDADD= libtestmucommon.la +TEST_PROGS += test-mu-container +test_mu_container_SOURCES= test-mu-container.c dummy.cc +test_mu_container_LDADD= libtestmucommon.la + # we need to use dummy.cc to enforce c++ linking... BUILT_SOURCES= \ dummy.cc diff --git a/lib/tests/test-mu-container.c b/lib/tests/test-mu-container.c new file mode 100644 index 00000000..475e3026 --- /dev/null +++ b/lib/tests/test-mu-container.c @@ -0,0 +1,83 @@ +/* -*-mode: c; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8 -*-*/ + +/* +** Copyright (C) 2014 Jakub Sitnicki +** +** This program is free software; you can redistribute it and/or modify it +** under the terms of the GNU General Public License as published by the +** Free Software Foundation; either version 3, or (at your option) any +** later version. +** +** This program is distributed in the hope that it will be useful, +** but WITHOUT ANY WARRANTY; without even the implied warranty of +** MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +** GNU General Public License for more details. +** +** You should have received a copy of the GNU General Public License +** along with this program; if not, write to the Free Software Foundation, +** Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +** +*/ + +#if HAVE_CONFIG_H +#include "config.h" +#endif /*HAVE_CONFIG_H*/ + +#include + +#include "test-mu-common.h" +#include "mu-container.h" + +static gboolean +container_has_children (const MuContainer *c) +{ + return c && c->child; +} + +static gboolean +container_is_sibling_of (const MuContainer *c, const MuContainer *sibling) +{ + const MuContainer *cur; + + for (cur = c; cur; cur = cur->next) { + if (cur == sibling) + return TRUE; + } + + return container_is_sibling_of (sibling, c); +} + +static void +test_mu_container_splice_children_when_parent_has_no_siblings (void) +{ + MuContainer *child, *parent, *root_set; + + child = mu_container_new (NULL, 0, "child"); + parent = mu_container_new (NULL, 0, "parent"); + parent = mu_container_append_children (parent, child); + + root_set = parent; + root_set = mu_container_splice_children (root_set, parent); + + g_assert (root_set != NULL); + g_assert (!container_has_children(parent)); + g_assert (container_is_sibling_of (root_set, child)); + + mu_container_destroy(parent); + mu_container_destroy(child); +} + +int +main (int argc, char *argv[]) +{ + g_test_init (&argc, &argv, NULL); + + g_test_add_func ("/mu-container/mu-container-splice-children-when-parent-has-no-siblings", + test_mu_container_splice_children_when_parent_has_no_siblings); + + g_log_set_handler (NULL, + G_LOG_LEVEL_MASK | G_LOG_FLAG_FATAL| G_LOG_FLAG_RECURSION, + (GLogFunc)black_hole, NULL); + + return g_test_run (); +} From bb0cc542b8f1bc00e210c5fecb53f1315fc0b81b Mon Sep 17 00:00:00 2001 From: Jakub Sitnicki Date: Wed, 13 Aug 2014 07:19:11 +0200 Subject: [PATCH 2/3] mu: Prune empty containers from the root set after splicing their children When the root set contains only one empty container with one child first promote the child container to the root set and only then remove the empty parent container so that the root set never goes empty. Also make mu_container_splice_children() do only one thing, that is promote one container's children to be another container's siblings. The resultant childless container is no longer removed by this function. Fixes #460. --- lib/mu-container.c | 2 -- lib/mu-container.h | 6 ++---- lib/mu-threader.c | 6 ++++-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/mu-container.c b/lib/mu-container.c index fbe612ff..f0743ac5 100644 --- a/lib/mu-container.c +++ b/lib/mu-container.c @@ -275,8 +275,6 @@ mu_container_splice_children (MuContainer *c, MuContainer *sibling) children = sibling->child; sibling->child = NULL; - c = mu_container_remove_sibling (c, sibling); - return mu_container_append_siblings (c, children); } diff --git a/lib/mu-container.h b/lib/mu-container.h index 9f2686a8..6cd76889 100644 --- a/lib/mu-container.h +++ b/lib/mu-container.h @@ -128,14 +128,12 @@ MuContainer* mu_container_remove_child (MuContainer *c, MuContainer *child); MuContainer* mu_container_remove_sibling (MuContainer *c, MuContainer *sibling); /** - * promote sibling's children to be this container's siblings and - * remove the sibling + * promote sibling's children to be this container's siblings * * @param c a container instance * @param sibling a sibling of this container * - * @return the container with the sibling's children promoted and the - * sibling itself removed + * @return the container with the sibling's children promoted */ MuContainer* mu_container_splice_children (MuContainer *c, diff --git a/lib/mu-threader.c b/lib/mu-threader.c index c42e166c..1bb4c0f5 100644 --- a/lib/mu-threader.c +++ b/lib/mu-threader.c @@ -433,10 +433,12 @@ prune_empty_containers (MuContainer *root_set) /* and prune the root_set itself... */ for (cur = root_set; cur; cur = cur->next) { - if (cur->flags & MU_CONTAINER_FLAG_DELETE) + if (cur->flags & MU_CONTAINER_FLAG_DELETE) { root_set = mu_container_remove_sibling (root_set, cur); - else if (cur->flags & MU_CONTAINER_FLAG_SPLICE) + } else if (cur->flags & MU_CONTAINER_FLAG_SPLICE) { root_set = mu_container_splice_children (root_set, cur); + root_set = mu_container_remove_sibling (root_set, cur); + } } return root_set; From c9cb27be11252cd975b5ee7b87dc95e85fc43716 Mon Sep 17 00:00:00 2001 From: Jakub Sitnicki Date: Thu, 14 Aug 2014 07:40:13 +0200 Subject: [PATCH 3/3] mu: Make mu_container_splice_grandchildren() do only one thing Don't kill the resultant childless container when promoting its children. This unifies the behavior of mu_container_splice_*() functions. --- lib/mu-container.c | 2 -- lib/mu-container.h | 2 +- lib/mu-threader.c | 6 ++++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/mu-container.c b/lib/mu-container.c index f0743ac5..7722b64a 100644 --- a/lib/mu-container.c +++ b/lib/mu-container.c @@ -290,8 +290,6 @@ mu_container_splice_grandchildren (MuContainer *parent, MuContainer *child) newchild = child->child; child->child=NULL; - mu_container_remove_child (parent, child); - return mu_container_append_children (parent, newchild); } diff --git a/lib/mu-container.h b/lib/mu-container.h index 6cd76889..5bc268b9 100644 --- a/lib/mu-container.h +++ b/lib/mu-container.h @@ -140,7 +140,7 @@ MuContainer* mu_container_splice_children (MuContainer *c, MuContainer *sibling); /** - * promote child's children to be parent's children and remove child + * promote child's children to be parent's children * * @param parent a container instance * @param child a child of this container diff --git a/lib/mu-threader.c b/lib/mu-threader.c index 1bb4c0f5..d32bbe7a 100644 --- a/lib/mu-threader.c +++ b/lib/mu-threader.c @@ -385,10 +385,12 @@ prune_maybe (MuContainer *c) MuContainer *cur; for (cur = c->child; cur; cur = cur->next) { - if (cur->flags & MU_CONTAINER_FLAG_DELETE) + if (cur->flags & MU_CONTAINER_FLAG_DELETE) { c = mu_container_remove_child (c, cur); - else if (cur->flags & MU_CONTAINER_FLAG_SPLICE) + } else if (cur->flags & MU_CONTAINER_FLAG_SPLICE) { c = mu_container_splice_grandchildren (c, cur); + c = mu_container_remove_child (c, cur); + } } g_return_val_if_fail (c, FALSE);