This is an automated email from the git hooks/post-receive script. guillem pushed a commit to branch master in repository dpkg.
View the commit online: https://git.dpkg.org/cgit/dpkg/dpkg.git/commit/?id=d5759dfebdf28ce28a6f243fa12fb03187fe0315 commit d5759dfebdf28ce28a6f243fa12fb03187fe0315 Author: Guillem Jover <guil...@debian.org> AuthorDate: Fri Dec 21 01:17:58 2018 +0100 u-a: Fix removal of obsolete slaves from the linked list The removal was not performed correctly, so depending on the order of the obsolete slave links, some would go unprocessed and thus not deleted from the list. Closes: #916799 Reported-by: Andreas Beckmann <a...@debian.org> --- debian/changelog | 2 ++ utils/t/update_alternatives.t | 53 ++++++++++++++++++++++++++++++++++++++++++- utils/update-alternatives.c | 10 ++++---- 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/debian/changelog b/debian/changelog index e5ffd7908..96a2b1312 100644 --- a/debian/changelog +++ b/debian/changelog @@ -19,6 +19,8 @@ dpkg (1.19.3) UNRELEASED; urgency=medium which implement the systemd readiness protocol for services. Closes: #910707 * update-alternatives: Add new --debug option. + * update-alternatives: Fix removal of obsolete slaves from the linked list. + Reported by Andreas Beckmann <a...@debian.org>. Closes: #916799 * Perl modules: - Dpkg::Changelog::Debian: Preserve modelines at EOF. Closes: #916056 Thanks to Chris Lamb <la...@debian.org> for initial test cases. diff --git a/utils/t/update_alternatives.t b/utils/t/update_alternatives.t index 84e2080da..c80edbad5 100644 --- a/utils/t/update_alternatives.t +++ b/utils/t/update_alternatives.t @@ -94,7 +94,7 @@ my @choices = ( ); my $nb_slaves = 4; plan tests => (4 * ($nb_slaves + 1) + 2) * 26 # number of check_choices - + 106; # rest + + 110; # rest sub cleanup { system("rm -rf $tmpdir && mkdir -p $admindir && mkdir -p $altdir"); @@ -515,3 +515,54 @@ ok(-f "$bindir/slave2", 'install + switching keeps real files installed as slave set_choice(1, params => ['--force']); ok(!-e "$bindir/slave2", 'forced switching w/o slave drops real files installed as slave links'); check_choice(1, 'manual', 'set --force replaces files with links'); + +# check disappearence of obsolete slaves (#916799) +cleanup(); +call_ua([ + '--install', "$bindir/test-obsolete", 'test-obsolete', "$paths{date}", '10', + '--slave', "$bindir/test-slave-a", 'test-slave-a', "$bindir/impl-slave-a", + '--slave', "$bindir/test-slave-b", 'test-slave-b', "$bindir/impl-slave-b", + '--slave', "$bindir/test-slave-c", 'test-slave-c', "$bindir/impl-slave-c", +], to_file => '/dev/null', error_to_file => '/dev/null'); + +my $content; +my $expected; + +$content = file_slurp("$admindir/test-obsolete"); +$expected = +"auto +$bindir/test-obsolete +test-slave-a +$bindir/test-slave-a +test-slave-b +$bindir/test-slave-b +test-slave-c +$bindir/test-slave-c + +$paths{date} +10 +$bindir/impl-slave-a +$bindir/impl-slave-b +$bindir/impl-slave-c + +"; +is($content, $expected, 'administrative file for non-obsolete slaves is as expected'); + +call_ua([ + '--install', "$bindir/test-obsolete", 'test-obsolete', "$paths{date}", '20', + '--slave', "$bindir/test-slave-c", 'test-slave-c', "$bindir/impl-slave-c", +], to_file => '/dev/null', error_to_file => '/dev/null'); + +$content = file_slurp("$admindir/test-obsolete"); +$expected = +"auto +$bindir/test-obsolete +test-slave-c +$bindir/test-slave-c + +$paths{date} +20 +$bindir/impl-slave-c + +"; +is($content, $expected, 'administrative file for obsolete slaves is as expected'); diff --git a/utils/update-alternatives.c b/utils/update-alternatives.c index 88db6e274..a57e97081 100644 --- a/utils/update-alternatives.c +++ b/utils/update-alternatives.c @@ -1373,7 +1373,8 @@ 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) { + sl = a->slaves; + while (sl) { bool has_slave = false; for (fs = a->choices; fs; fs = fs->next) { @@ -1393,10 +1394,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; } } -- Dpkg.Org's dpkg