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

[Seth Arnold]

On Wed, Jan 18, 2017 at 10:37:44PM +0100, Christian Boltz wrote:
> 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()

Hi Christian, this is great, thanks!

Acked-by: Seth Arnold <seth.arnold at canonical.com>

Thanks

> 
> 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)
>  
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20170118/1935475f/attachment.pgp>