The following commit has been merged in the master branch:
commit a82c44c4ae20fdff7c9b083e75a1b28a528b59c6
Author: Raphael Hertzog <hert...@debian.org>
Date:   Sat Feb 21 21:08:01 2009 +0100

    update-alternatives: stricter validation of --install parameters
    
    Fail if --install tries to reuse links owned by other alternatives, or
    if the alternative type (slave/master) doesn't match the information
    already available. Closes: #342566
    Modify test-suite to verify this behaviour.

diff --git a/ChangeLog b/ChangeLog
index 59f5a57..c3682fb 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2009-02-22  Raphael Hertzog  <hert...@debian.org>
 
+       * scripts/update-alternatives.pl: Checks that --install does not
+       reuse alternatives or links in a way that is not compatible with
+       the current setup.
+       * scripts/t/900_update_alternatives.t: Add corresponding tests in
+       the test suite.
+
+2009-02-22  Raphael Hertzog  <hert...@debian.org>
+
        * scripts/update-alternatives.pl: Add logging to
        /var/log/dpkg.log.
 
diff --git a/debian/changelog b/debian/changelog
index 49d0f70..df96417 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -157,6 +157,9 @@ dpkg (1.15.0) UNRELEASED; urgency=low
     implement new features on top of it:
     - the --config output is now sorted. Closes: #437060
     - it now logs information to /var/log/dpkg.log. Closes: #445270
+    - it forbids reusing master alternative as slave and vice-versa.
+      Closes: #342566
+    - it forbids reusing alternative links managed by other alternatives
 
   [ Pierre Habouzit ]
   * Add a --query option to update-alternatives. Closes: #336091, #441904
diff --git a/scripts/t/900_update_alternatives.t 
b/scripts/t/900_update_alternatives.t
index ad7bd89..4d1004b 100644
--- a/scripts/t/900_update_alternatives.t
+++ b/scripts/t/900_update_alternatives.t
@@ -53,7 +53,7 @@ my @choices = (
 );
 my $nb_slaves = 2;
 plan tests => (4 * ($nb_slaves + 1) + 2) * 18 # number of check_choices
-               + 39;                         # rest
+               + 44;                         # rest
 
 sub cleanup {
     system("rm -rf $srcdir/t.tmp/ua && mkdir -p $admindir && mkdir -p 
$altdir");
@@ -264,5 +264,22 @@ cleanup();
 system("echo garbage > $admindir/generic-test");
 install_choice(0, error_to_file => "/dev/null", expect_failure => 1);
 
-# TODO: add checks for invalid values of parameters, add checks for
-# usage of links as both master and slave, install in non-existing dir
+# test invalid usages
+cleanup();
+install_choice(0);
+# try to install a slave alternative as new master
+call_ua(["--install", "$bindir/testmaster", "slave1", "/bin/date", "10"],
+        expect_failure => 1, to_file => "/dev/null", error_to_file => 
"/dev/null");
+# try to install a master alternative as slave
+call_ua(["--install", "$bindir/testmaster", "testmaster", "/bin/date", "10",
+        "--slave", "$bindir/testslave", "generic-test", "/bin/true" ],
+       expect_failure => 1, to_file => "/dev/null", error_to_file => 
"/dev/null");
+# try to reuse links in master alternative
+call_ua(["--install", "$bindir/slave1", "testmaster", "/bin/date", "10"],
+        expect_failure => 1, to_file => "/dev/null", error_to_file => 
"/dev/null");
+# try to reuse links in slave alternative
+call_ua(["--install", "$bindir/testmaster", "testmaster", "/bin/date", "10",
+        "--slave", "$bindir/generic-test", "testslave", "/bin/true" ],
+       expect_failure => 1, to_file => "/dev/null", error_to_file => 
"/dev/null");
+
+# TODO: add checks for invalid values of parameters, install in non-existing 
dir
diff --git a/scripts/update-alternatives.pl b/scripts/update-alternatives.pl
index 5076d84..2f5bb35 100755
--- a/scripts/update-alternatives.pl
+++ b/scripts/update-alternatives.pl
@@ -108,6 +108,47 @@ while (@ARGV) {
 badusage(_g("need --display, --query, --list, --config, --set, --install," .
             "--remove, --all, --remove-all or --auto")) unless $action;
 
+# Load infos about all alternatives to be able to check for mistakes
+my %ALL;
+foreach my $alt_name (get_all_alternatives()) {
+    my $alt = Alternative->new($alt_name);
+    next unless $alt->load("$admdir/$alt_name", 1);
+    $ALL{objects}{$alt_name} = $alt;
+    $ALL{links}{$alt->link()} = $alt_name;
+    $ALL{parent}{$alt_name} = $alt_name;
+    foreach my $slave ($alt->slaves()) {
+        $ALL{links}{$alt->slave_link($slave)} = $slave;
+        $ALL{parent}{$slave} = $alt_name;
+    }
+}
+# Check that caller don't mix links between alternatives and don't mix
+# alternatives between
+if ($action eq "install") {
+    my ($name, $link) = ($inst_alt->name(), $inst_alt->link());
+    if (exists $ALL{parent}{$name} and $ALL{parent}{$name} ne $name) {
+        badusage(_g("Alternative %s can't be master: %s"), $name,
+                 sprintf(_g("it is a slave of %s"), $ALL{parent}{$name}));
+    }
+    if (exists $ALL{links}{$link} and $ALL{links}{$link} ne $name) {
+        badusage(_g("Alternative link %s is already managed by %s."),
+                 $link, $ALL{parent}{$ALL{links}{$link}});
+    }
+    foreach my $slave ($inst_alt->slaves()) {
+        $link = $inst_alt->slave_link($slave);
+        if (exists $ALL{parent}{$slave} and $ALL{parent}{$slave} ne $name) {
+            badusage(_g("Alternative %s can't be slave of %s: %s"),
+                     $slave, $name, ($ALL{parent}{$slave} eq $slave) ?
+                        _g("it is a master alternative.") :
+                        sprintf(_g("it is a slave of %s"), 
$ALL{parent}{$slave})
+                     );
+        }
+        if (exists $ALL{links}{$link} and $ALL{links}{$link} ne $slave) {
+            badusage(_g("Alternative link %s is already managed by %s."),
+                     $link, $ALL{parent}{$ALL{links}{$link}});
+        }
+    }
+}
+
 # Handle actions
 if ($action eq 'all') {
     config_all();
@@ -373,12 +414,16 @@ sub set_action {
     }
 }
 
-sub config_all {
+sub get_all_alternatives {
     opendir(ADMINDIR, $admdir)
         or quit(_g("can't readdir %s: %s"), $admdir, $!);
-    my @filenames = grep !/^\.\.?$/, readdir ADMINDIR;
+    my @filenames = grep { !/^\.\.?$/ and !/\.dpkg-tmp$/ } (readdir(ADMINDIR));
     close(ADMINDIR);
-    foreach my $name (@filenames) {
+    return sort @filenames;
+}
+
+sub config_all {
+    foreach my $name (get_all_alternatives()) {
         system "$0 $skip --config $name";
         exit $? if $?;
         print "\n";

-- 
dpkg's main repository


-- 
To UNSUBSCRIBE, email to debian-dpkg-cvs-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to