[apparmor] [PATCH 2/2] utils: Clean up str_to_mode()

[Christian Boltz]

Hello,

Am Mittwoch, 23. April 2014 schrieb Tyler Hicks:
> The first conditional around string being set is not needed. If string
> is not set, the while loop will be skipped and mode will be returned.
> 
> The variable tmp was being overloaded by being the regex search result
> and then being reassigned to be the first match group in the regex
> search result. This patch keeps tmp as the regex search result and
> then uses mode_char to represent the first match group of the search.
> 
> Group the search and replace actions together at the beginning of the
> loop and group the mode character processing at the end of the loop.
> 
> Finally, remove the unnecessary check of tmp (now mode_char) before
> calling MODE_HASH.get(tmp, False). If tmp is None or '', get() will
> do the right thing and return False.
> 
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
> ---
>  utils/apparmor/aamode.py | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/utils/apparmor/aamode.py b/utils/apparmor/aamode.py
> index a9fd307..6f64e45 100644
> --- a/utils/apparmor/aamode.py
> +++ b/utils/apparmor/aamode.py
> @@ -87,16 +87,16 @@ def str_to_mode(string):
> 
>  def sub_str_to_mode(string):
>      mode = set()
> -    if not string:
> -        return mode
> +
>      while string:
>          tmp = MODE_MAP_RE.search(string)
>          if not tmp:

Would a warning like "unknown mode %s"%string in the   if not tmp:  
branch be a good idea? Silently ignoring the remaining part doesn't 
sound like the best solution. (I know that warning won't be too helpful 
because the function doesn't know where string comes from, but it's 
still more helpful than silently ignoring stuff.)

>              break
> -        tmp = tmp.groups()[0]
>          string = MODE_MAP_RE.sub('', string, 1)
> -        if tmp and MODE_HASH.get(tmp, False):
> -            mode |= MODE_HASH[tmp]
> +
> +        mode_char = tmp.groups()[0]
> +        if MODE_HASH.get(mode_char, False):
> +            mode |= MODE_HASH[mode_char]
>          else:
>              pass

Nice cleanup!

With or without the warning,
Acked-by: Christian Boltz <apparmor at cboltz.de>


Regards,

Christian Boltz
-- 
> Entscheiden was du brauchst musst du vorher, ein absolut stabiles 
> System(das ist stable bei Debian unbestritten) oder etwas aktuelles
Das Ding ist so stabil, daß ein "clamscan" sogar zu 100% reproduzierbar
einfriert -- seit einem Dreivierteljahr.
[Uwe Driessen und Peer Heinlein in postfixbuch-users]