Control: severity -1 normal
Hi!
On Tue, 2018-12-18 at 19:28:35 +0100, Andreas Beckmann wrote:
> Package: dpkg
> Version: 1.19.2
> Severity: serious
> Control: found -1 1.18.25
> there is a problem with update-alternatives not properly deregistering
> obsolete slaves:
> ==========
> #!/bin/sh
>
> update-alternatives --version
> update-alternatives --remove-all foobar
>
> set -x
> touch /foo /foo.1
> touch /bar /bar.1
> update-alternatives --display foobar
> update-alternatives --install /foobar foobar /foo 23 --slave /foobar.1
> foobar-1 /foo.1 --slave /barfoo barfoo /bar --slave /barfoo.1 barfoo-1 /bar.1
> update-alternatives --display foobar
> rm /bar /bar.1
> update-alternatives --install /foobar foobar /foo 42 --slave /foobar.1
> foobar-1 /foo.1
> update-alternatives --display foobar
> update-alternatives --install /foobar foobar /foo 42 --slave /foobar.1
> foobar-1 /foo.1
> update-alternatives --display foobar
> ==========
Thanks for the reproducer, much appreciated!
> # ./foobar.sh
[…]
> So this is reproducible in stretch, too, but not in jessie.
It is actually reproducible in both, it is just not obviosly visible
in jessie. What made this visible was the change from commit
55661248cc3028446b9e73eb17e41fa12aad2b54 which started printing the
master and slave links on --display. But if you check the database
(/var/lib/dpkg/alternatives/foobar) you'll see they are present even
in 1.17.x and earlier. I've not checked whether the perl implementation
suffered the same problem, but I assume it did not, and it seems this
was a regression from the C rewrite.
> The problem does not occur if there is only a single broken slave to be
> removed.
Right, because this is a problem with the way the slave links are
removed from the single linked list.
I've lowered the severity, because this is a self-healing problem, and
on each new call the remaining obsolete slave links will get removed,
even if slowly. :)
The attached patch should fix it.
Thanks,
Guillem
diff --git i/utils/update-alternatives.c w/utils/update-alternatives.c
index 88db6e274..79dbd10c7 100644
--- i/utils/update-alternatives.c
+++ w/utils/update-alternatives.c
@@ -1373,7 +1373,7 @@ alternative_save(struct alternative *a)
/* Cleanup unused slaves before writing admin file. */
sl_prev = NULL;
- for (sl = a->slaves; sl; sl_prev = sl, sl = sl->next) {
+ for (sl = a->slaves; sl; ) {
bool has_slave = false;
for (fs = a->choices; fs; fs = fs->next) {
@@ -1393,10 +1393,11 @@ alternative_save(struct alternative *a)
else
a->slaves = sl->next;
sl_rm = sl;
- sl = sl_prev ? sl_prev : a->slaves;
+ sl = sl->next;
slave_link_free(sl_rm);
- if (!sl)
- break; /* No other slave left. */
+ } else {
+ sl_prev = sl;
+ sl = sl->next;
}
}
