Hello,
the review for r40 is attached. I also included the r34 [v2] comments,
so you can skip the mail with the updated r34 review ;-)
@John: it also includes a question for you (the same I asked on IRC, but
you didn't respond yet ;-)
Regards,
Christian Boltz
--
Sagt mal ehrlich: Ist mein Rechner geisteskrank????
[Harald Katzer in suse-linux]
------------------------------------------------------------
revno: 40
committer: Kshitij Gupta <kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Sat 2013-08-10 12:46:22 +0530
message:
fixes from rev 32..39 and fixed(?) flags=(..)thing
=== modified file 'Testing/common_test.py'
--- Testing/common_test.py 2013-08-05 13:25:34 +0000
+++ Testing/common_test.py 2013-08-10 07:16:22 +0000
@@ -4,72 +4,23 @@
+ tests=apparmor.config.Config('ini')
+ tests.CONF_DIR = '.'
+ regex_tests = tests.read_config('regex_tests.ini')
+ for regex in regex_tests.sections():
+ parsed_regex = re.compile(apparmor.common.convert_regexp(regex))
+ for regex_testcase in regex_tests.options(regex):
+ self.assertEqual(bool(parsed_regex.search(regex_testcase)),
eval(regex_tests[regex][regex_testcase]), 'Incorrectly Parsed regex')
# the error message should print out which regex failed
# besides that, this + regex_tests.ini are much better readable :-)
=== modified file 'Tools/aa-logprof.py'
--- Tools/aa-logprof.py 2013-08-07 09:13:17 +0000
+++ Tools/aa-logprof.py 2013-08-10 07:16:22 +0000
@@ -7,9 +7,9 @@
import argparse
if sys.version_info < (3,0):
- os.environ['PATH'] = '/bin/:/sbin/:/usr/bin/:/usr/sbin'
+ os.environ['AAPATH'] = '/bin/:/sbin/:/usr/bin/:/usr/sbin'
else:
- os.environb.putenv('PATH', '/bin/:/sbin/:/usr/bin/:/usr/sbin')
+ os.environb.putenv('AAPATH', '/bin/:/sbin/:/usr/bin/:/usr/sbin')
=== modified file 'apparmor/aa.py'
--- apparmor/aa.py 2013-08-09 11:19:01 +0000
+++ apparmor/aa.py 2013-08-10 07:16:22 +0000
@@ -215,7 +215,7 @@
def which(file):
"""Returns the executable fullpath for the file, None otherwise"""
- env_dirs = os.getenv('PATH').split(':')
+ env_dirs = os.getenv('AAPATH').split(':')
# renaming the variable doesn't change much ;-)
# the question is if we want to override PATH or if we should use the existing
dirs in PATH
# (which users might called expected behaviour, but could also include a small
security risk)
# John?
for env_dir in env_dirs:
env_path = env_dir + '/' + file
# Test if the path is executable or not
@@ -1751,6 +1748,7 @@
def ask_the_questions():
found = None
+ print(log_dict)
# debug code?
for aamode in sorted(log_dict.keys()):
# Describe the type of changes
if aamode == 'PERMITTING':
@@ -2955,10 +2953,9 @@
repo_data = None
parsed_profiles = []
initial_comment = ''
- RE_PROFILE_START =
re.compile('^(("??\/.+?"??)|(profile\s+("??.+?"??)))\s+((flags=)?\((.+)\)\s+)*\{\s*(#.*)?$')
+ RE_PROFILE_START =
re.compile('^(("??\/.+?"??)|(profile\s+("??.+?"??)))\s+((flags=)?\((.+)\)\s+)?\{\s*(#.*)?$')
^
# regex_bin_flag does not have this \ - which of them is correct? ;-)
# (it would be even better to share the regex)
@@ -3373,12 +3361,12 @@
else:
print('Ignored: New definition for variable for:',list_var,'=',
value, 'operation was:',var_operation,'old value=', var[list_var])
pass
- #raise AppArmorException('An existing variable redefined')
+ #raise AppArmorException('An existing variable redefined: %s'
%list_var)
# (r35 comment)
# variable redefinition should be an error, not a warning
# (apparmor_parser also errors out IIRC)
else:
# (r34 [v2] comment)
# a comment saying "else" means "+=" would be nice
# or, better, explicitely check for "+=" and add an else: that errors out
saying "invalid var_operation"
if var.get(list_var, False):
var[list_var] = set(var[list_var] + vlist)
else:
- raise AppArmorException('An existing variable redefined')
+ raise AppArmorException('An existing variable redefined: %s'
%list_var)
# (r34 [v2] comment)
# wrong error message, should be something like "attemped to append to
non-existing variable"
=== modified file 'apparmor/common.py'
--- apparmor/common.py 2013-08-09 19:47:00 +0000
+++ apparmor/common.py 2013-08-10 07:16:22 +0000
@@ -165,13 +165,12 @@
# new_reg = new_reg.replace('}', '}')
# new_reg = new_reg.replace(',', '|')
- while re.search('{.*,.*}', new_reg):
- match = re.search('(.*){(.*),(.*)}(.*)', new_reg).groups()
+ while re.search('{[^}]*,[^}]*}', new_reg):
+ match = re.search('(.*){([^}]*)}(.*)', new_reg).groups()
# I'd use the same regex for while and match (hint: use the regex from match)
and store it in a variable
# I'd also use ^...$ to be on the safe side
@@ -190,18 +189,25 @@
class DebugLogger:
+ def __init__(self, module_name=__name__):
self.debugging = False
+ self.debug_level = logging.DEBUG
if os.getenv('LOGPROF_DEBUG', False):
- self.debugging = True
+ self.debugging = os.getenv('LOGPROF_DEBUG')
# what happens if LOGPROF_DEBUG contains '12 monkeys' or 'I hate logs' ?
# hint: convert LOGPROF_DEBUG to integer to be on the safe side, and check for
valid values (0..3)
+ if self.debugging == 1:
+ debug_level = logging.ERROR
+ elif self.debug_level == 2:
+ debug_level = logging.INFO
# what about loglevel == 3 for DEBUG ?
+ #logging.basicConfig(filename='/var/log/apparmor/logprof.log',
level=self.debug_level, format='%(asctime)s - %(name)s - %(message)s\n')
+ logging.basicConfig(filename='/home/kshitij/logprof.log',
level=self.debug_level, format='%(asctime)s - %(name)s - %(message)s\n')
# golden rules of bad programming, rule 10 *SCNR*
vim:ft=diff
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
