On Sun, Aug 07, 2016 at 04:57:44PM +0200, Christian Boltz wrote: > 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. > > > I propose this patch 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.
Nice find. Acked-by: Seth Arnold <[email protected]> Acked for both branches. Thanks > > > [ 01-delete_duplicates-dont-modify-list-in-loop.diff ] > > --- utils/apparmor/rule/__init__.py 2016-07-31 19:12:31.537453276 +0200 > +++ utils/apparmor/rule/__init__.py 2016-08-07 16:32:19.435814124 +0200 > @@ -431,10 +431,13 @@ > > # 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 2016-07-31 19:12:31.517453376 +0200 > +++ utils/test/cleanprof_test.out 2016-08-07 16:37:22.402324691 +0200 > @@ -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, > > allow /home/*/** r, > --- utils/test/test-capability.py 2015-11-19 17:42:26.325879118 +0100 > +++ utils/test/test-capability.py 2016-08-07 16:40:33.097385534 +0200 > @@ -817,7 +817,6 @@ > 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 @@ > 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)) >
signature.asc
Description: PGP signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
