delete_duplicates(): don't modify self.rules while looping over it

By calling self.delete() inside the delete_duplicates() loop, the
self.rules list was modified. This resulted in some rules not being
checked and therefore (some, not all) superfluous rules not being
removed.

This patch switches to a temporary variable to loop over, and rebuilds
self.rules with the rules that are not superfluous.

This also fixes some strange issues already marked with a "Huh?" comment
in the tests.


Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk and 2.10.

Note that in 2.10 cleanprof_test.* doesn't contain a ptrace rule,
therefore the cleanprof_test.out change doesn't make sense for 2.10.

utils/apparmor/rule/__init__.py

@@ -397,10 +397,13 @@ class BaseRuleset(object):
 
         # delete rules that are covered by include files
         if include_rules:
-            for rule in self.rules:
-                if include_rules.is_covered(rule, True, True):
-                    self.delete(rule)
+            oldrules = self.rules
+            self.rules = []
+            for rule in oldrules:
+                if include_rules.is_covered(rule, True, False):
                     deleted += 1
+                else:
+                    self.rules.append(rule)
 
         # de-duplicate rules inside the profile
         deleted += self.delete_in_profile_duplicates()

utils/test/cleanprof_test.out

@@ -16,8 +16,6 @@
 
   signal set=(abrt alrm bus chld fpe hup ill int kill pipe quit segv stkflt term trap usr1 usr2),
 
-  ptrace tracedby,
-
   unix (receive) type=dgram,
 
   /home/*/** r,

utils/test/test-capability.py

@@ -817,7 +817,6 @@ class CapabilityDeleteTest(AATest):
             inc.add(CapabilityRule.parse(rule))
 
         expected_raw = [
-            '  allow capability sys_admin,',  # XXX huh? should be deleted!
             '  deny capability chgrp, # example comment',
             '',
         ]
@@ -825,11 +824,9 @@ class CapabilityDeleteTest(AATest):
         expected_clean = [
             '  deny capability chgrp, # example comment',
             '',
-            '  allow capability sys_admin,',  # XXX huh? should be deleted!
-            '',
         ]
 
-        self.assertEqual(self.ruleset.delete_duplicates(inc), 1)
+        self.assertEqual(self.ruleset.delete_duplicates(inc), 2)
         self.assertEqual(expected_raw, self.ruleset.get_raw(1))
         self.assertEqual(expected_clean, self.ruleset.get_clean(1))