[apparmor] [patch] [6/7] make log_dict a parameter of ask_the_questions()

[Christian Boltz]

Hello,

Am Dienstag, 17. Januar 2017, 13:04:05 CET schrieb Seth Arnold:
> I'm really not a fan of how the local parameter 'log_dict' now shadows
> the global variable 'log_dict'. This is a recipe for future trouble.

As discussed on IRC yesterday, here's an updated patch which gets rid of the global variable 'log_dict'.



[patch] [6/7] make log_dict a parameter of ask_the_questions()

This allows to hand over any source instead of using the global variable.

Now that the function expects its input as parameter,  get rid of the
global log_dict, which means
- change collapse_log() to initialize log_dict as local variable and
  return it
- change do_logprof_pass() to catch collapse_log()'s return value and
  hand it over to ask_the_questions()
- drop all references to the global log_dict variable
- update test-libapparmor-test_multi to follow the changes

Also fix an if condition that would fail if aa[profile][hat] does not
exist - get() defaults to None if the requested item doesn't exist, and
None.get('file') will raise an Exception.


[ 06-ask_the_questions-param.diff ]

=== modified file ./utils/apparmor/aa.py
--- utils/apparmor/aa.py        2017-01-18 22:13:06.913279710 +0100
+++ utils/apparmor/aa.py        2017-01-18 22:10:44.509961451 +0100
@@ -117,7 +117,6 @@
 seen = hasher()  # dir()
 profile_changes = hasher()
 prelog = hasher()
-log_dict = hasher()  # dict()
 changed = dict()
 created = []
 skip = hasher()
@@ -1516,7 +1517,7 @@
 
     return globs
 
-def ask_the_questions():
+def ask_the_questions(log_dict):
     for aamode in sorted(log_dict.keys()):
         # Describe the type of changes
         if aamode == 'PERMITTING':
@@ -1544,7 +1545,7 @@
 
             for hat in hats:
 
-                if not aa[profile].get(hat).get('file'):
+                if not aa[profile].get(hat, {}).get('file'):
                     if aamode != 'merge':
                         # Ignore log events for a non-existing profile or child profile. Such events can occour
                         # after deleting a profile or hat manually, or when processing a foreign log.
@@ -1621,7 +1622,6 @@
 
                 for ruletype in ruletypes:
                     for rule_obj in log_dict[aamode][profile][hat][ruletype].rules:
-                        # XXX aa-mergeprof also has this code - if you change it, keep aa-mergeprof in sync!
 
                         if is_known_rule(aa[profile][hat], ruletype, rule_obj):
                             continue
@@ -1754,7 +1754,6 @@
 
                             else:
                                 done = False
-                    # END of code (mostly) shared with aa-mergeprof
 
 def selection_to_rule_obj(rule_obj, selection):
     rule_type = type(rule_obj)
@@ -1911,7 +1910,6 @@
 #    aa = hasher()
 #    profile_changes = hasher()
 #     prelog = hasher()
-#     log_dict = hasher()
 #     changed = dict()
 #    skip = hasher()  # XXX global?
 #    filelist = hasher()
@@ -1943,9 +1941,9 @@
     for pid in sorted(profile_changes.keys()):
         set_process(pid, profile_changes[pid])
 
-    collapse_log()
+    log_dict = collapse_log()
 
-    ask_the_questions()
+    ask_the_questions(log_dict)
 
     if aaui.UI_mode == 'yast':
         # To-Do
@@ -2151,6 +2149,7 @@
     process.close()
 
 def collapse_log():
+    log_dict = hasher()
     for aamode in prelog.keys():
         for profile in prelog[aamode].keys():
             for hat in prelog[aamode][profile].keys():
@@ -2231,6 +2230,8 @@
                             if not is_known_rule(aa[profile][hat], 'signal', signal_event):
                                 log_dict[aamode][profile][hat]['signal'].add(signal_event)
 
+    return log_dict
+
 def is_skippable_file(path):
     """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()

--- utils/test/test-libapparmor-test_multi.py   2016-11-01 21:57:42.345480000 +0100
+++ utils/test/test-libapparmor-test_multi.py   2017-01-18 22:17:11.028069827 +0100
@@ -214,7 +214,6 @@
         apparmor.aa.log = dict()
         apparmor.aa.aa = apparmor.aa.hasher()
         apparmor.aa.prelog = apparmor.aa.hasher()
-        apparmor.aa.log_dict = apparmor.aa.hasher()
 
         profile = parsed_event['profile']
         hat = profile
@@ -229,12 +229,12 @@
         for root in log:
             apparmor.aa.handle_children('', '', root)  # interactive for exec events!
 
-        apparmor.aa.collapse_log()
+        log_dict = apparmor.aa.collapse_log()
 
         apparmor.aa.filelist = apparmor.aa.hasher()
         apparmor.aa.filelist[profile_dummy_file]['profiles'][profile] = True
 
-        new_profile = apparmor.aa.serialize_profile(apparmor.aa.log_dict[aamode][profile], profile, None)
+        new_profile = apparmor.aa.serialize_profile(log_dict[aamode][profile], profile, None)
 
         expected_profile = read_file('%s.profile' % params)
 


Regards,

Christian Boltz
-- 
Wenn die Dinge bei euch so schlimm seid, dass ihr einen Code of Conduct
braucht, dann ist eure Gruppe wie ein Misthaufen mit einem Leinentuch
drüber. Das Problem geht nicht weg, wenn ihr da ein Leinentuch
drüberhängt. Löst das Problem. Dann braucht ihr auch keinen Code of
Conduct mehr. [http://blog.fefe.de/?ts=a6e90c0a]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20170118/47e4cb34/attachment.pgp>