Hello, [resent without linebreaks in the patch]
Am Dienstag, 3. Februar 2015 schrieb Christian Boltz: > Am Donnerstag, 25. Dezember 2014 schrieb Kshitij Gupta: > > On Thu, Dec 25, 2014 at 6:02 AM, Christian Boltz wrote: > > > Am Mittwoch, 24. Dezember 2014 schrieb Kshitij Gupta: > > > > On Sun, Dec 7, 2014 at 2:49 AM, Christian Boltz wrote: > <big snip, scroll down for updated patch> > > > > I'd also be interested in some numbers - how long do the following > > > variants take? > > > > > > a) the "old" code we currently have in bzr (hint: it does 4 > > > re.search) > > > > 0.185 s for 1000 (x13 in each for all cases) calls to functions > > > > > b) with my patch applied (only one re.search, so I'd expect it to > > > be > > > > > > faster) > > > > 0.160 s for 1000 (x13 in each for all cases) calls to functions > > That difference isn't too big. > > >> c) with my patch applied + using re.compile for skippable_files > >> > > > c1) with re.compile inside the function (= one re.compile per > > > file) > > > > 0.144 s for 1000 (x13 in each for all cases) calls to functions > > So re.compile already improves things for a regex used only once. > > > > c2) with re.compile outside the function (= only one re.compile > > > in total) > > > > 0.065 s for 1000 (x13 in each for all cases) calls to functions > > That's a clear sign that we should use re.compile() everywhere, > ideally in the global area (outside of any functions) for often-used > regexes so that each regex is compiled only once. > > > d) with the code using os.path.* you proposed > > > > Using os.path.* and in operator: > > 0.082 s for 1000 (x13 in each for all cases) calls to functions > > > > Using os.path.basename followed by your suggested endswith for > > basename: 0.052 s for 1000 (x13 in each for all cases) calls to > > functions > > This variant is fast, easily readable, avoids the escaping we would > need for a regex and avoids having something (the compiled regex) > "polluting" the module namespace, so it's the way to go :-) > > > I've attach the test scripts, separate files for all 6 cases. > > Thanks, and thanks for all the testing! > > Here's the updated patch, based on your "ext6.py". I only changed the > aa.py part, the testcases didn't change since v1. > > > > Update is_skippable_file() to match all extensions that are listed in > libapparmor _aa_is_blacklisted() - some extensions were missing in the > python code. > > Also make the code more readable and add some testcases. > > Notes: > - the original code additionally ignored *.swp. I didn't include that > - *.swp looks like vim swap files which are also dot files > - the python code ignores README files, but the C code doesn't > (do we need to add README in the C code?) > > [ is_skippable_file.diff ] === modified file 'utils/apparmor/aa.py' --- utils/apparmor/aa.py 2015-01-30 20:08:17 +0000 +++ utils/apparmor/aa.py 2015-02-03 18:12:37 +0000 @@ -2539,14 +2539,22 @@ else: return False -# rpm backup files, dotfiles, emacs backup files should not be processed -# The skippable files type needs be synced with apparmor initscript + def is_skippable_file(path): - """Returns True if filename matches something to be skipped""" - if (re.search('(^|/)\.[^/]*$', path) or re.search('\.rpm(save|new)$', path) - or re.search('\.dpkg-(old|new)$', path) or re.search('\.swp$', path) - or path[-1] == '~' or path == 'README'): - return True + """Returns True if filename matches something to be skipped (rpm or dpkg backup files, hidden files etc.) + The list of skippable files needs to be synced with apparmor initscript and libapparmor _aa_is_blacklisted() + path: filename (with or without directory)""" + + basename = os.path.basename(path) + + if not basename or basename[0] == '.' or basename == 'README': + return True + + skippable_suffix = ('.dpkg-new', '.dpkg-old', '.dpkg-dist', '.dpkg-bak', '.rpmnew', '.rpmsave', '.orig', '.rej', '~') + if basename.endswith(skippable_suffix): + return True + + return False def is_skippable_dir(path): if re.search('(disable|cache|force-complain|lxc)', path): === modified file 'utils/test/test-aa.py' --- utils/test/test-aa.py 2014-11-27 22:20:26 +0000 +++ utils/test/test-aa.py 2015-02-03 17:57:25 +0000 @@ -15,7 +15,7 @@ import tempfile from common_test import write_file -from apparmor.aa import check_for_apparmor +from apparmor.aa import check_for_apparmor, is_skippable_file class AaTest_check_for_apparmor(unittest.TestCase): FILESYSTEMS_WITH_SECURITYFS = 'nodev\tdevtmpfs\nnodev\tsecurityfs\nnodev\tsockfs\n\text3\n\text2\n\text4' @@ -70,6 +70,48 @@ mounts = write_file(self.tmpdir, 'mounts', self.MOUNTS_WITH_SECURITYFS % self.tmpdir) self.assertEqual('%s/security/apparmor' % self.tmpdir, check_for_apparmor(filesystems, mounts)) +class AaTest_is_skippable_file(unittest.TestCase): + def test_not_skippable_01(self): + self.assertFalse(is_skippable_file('bin.ping')) + def test_not_skippable_02(self): + self.assertFalse(is_skippable_file('usr.lib.dovecot.anvil')) + def test_not_skippable_03(self): + self.assertFalse(is_skippable_file('bin.~ping')) + def test_not_skippable_04(self): + self.assertFalse(is_skippable_file('bin.rpmsave.ping')) + def test_not_skippable_05(self): + # normally is_skippable_file should be called without directory, but it shouldn't hurt too much + self.assertFalse(is_skippable_file('/etc/apparmor.d/bin.ping')) + def test_not_skippable_06(self): + self.assertFalse(is_skippable_file('bin.pingrej')) + + def test_skippable_01(self): + self.assertTrue(is_skippable_file('bin.ping.dpkg-new')) + def test_skippable_02(self): + self.assertTrue(is_skippable_file('bin.ping.dpkg-old')) + def test_skippable_03(self): + self.assertTrue(is_skippable_file('bin.ping..dpkg-dist')) + def test_skippable_04(self): + self.assertTrue(is_skippable_file('bin.ping..dpkg-bak')) + def test_skippable_05(self): + self.assertTrue(is_skippable_file('bin.ping.rpmnew')) + def test_skippable_06(self): + self.assertTrue(is_skippable_file('bin.ping.rpmsave')) + def test_skippable_07(self): + self.assertTrue(is_skippable_file('bin.ping.orig')) + def test_skippable_08(self): + self.assertTrue(is_skippable_file('bin.ping.rej')) + def test_skippable_09(self): + self.assertTrue(is_skippable_file('bin.ping~')) + def test_skippable_10(self): + self.assertTrue(is_skippable_file('.bin.ping')) + def test_skippable_11(self): + self.assertTrue(is_skippable_file('')) # empty filename + def test_skippable_12(self): + self.assertTrue(is_skippable_file('/etc/apparmor.d/')) # directory without filename + def test_skippable_13(self): + self.assertTrue(is_skippable_file('README')) + if __name__ == '__main__': unittest.main(verbosity=2) Regards, Christian Boltz -- "Bei mir" läuft KDE gar nicht. Völlig korrekt. Logisch. Aber sinnfrei. [David Haller in opensuse-de] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor