[apparmor] [patch] aa.py / ask_the_question - simplify duplicate option prevention

[Kshitij Gupta]

Hello,

On Sunday, August 24, 2014 04:44:40 PM Christian Boltz wrote:
> Hello,
> 
> this patch adds a add_to_options() helper function to aa.py which
> - adds newpath to options if it's not already there
> - returns the updated options and the index of newpath
> 
> This removes duplicated code for CMD_GLOB and CMD_GLOBEXT in
> ask_the_question()
> 
> It also adds duplicate prevention to CMD_NEW.
> 
> 
> === modified file 'utils/apparmor/aa.py'
> --- utils/apparmor/aa.py        2014-08-17 16:16:33 +0000
> +++ utils/apparmor/aa.py        2014-08-24 14:33:35 +0000
> @@ -1921,30 +1931,19 @@
>                                                  continue
> 
>                                          user_globs.append(ans)
> -                                        options.append(ans)
> -                                        default_option = len(options)
> +                                        options, default_option =
> add_to_options(options, ans)
> 
>                              elif ans == 'CMD_GLOB':
>                                  newpath = options[selected].strip()
>                                  if not re_match_include(newpath):
>                                      newpath = glob_path(newpath)
> -
> -                                    if newpath not in options:
> -                                        options.append(newpath)
> -                                        default_option = len(options)
> -                                    else:
> -                                        default_option =
> options.index(newpath) + 1 +                                    options,
> default_option = add_to_options(options, newpath)
> 
>                              elif ans == 'CMD_GLOBEXT':
>                                  newpath = options[selected].strip()
>                                  if not re_match_include(newpath):
>                                      newpath = glob_path_withext(newpath)
> -
> -                                    if newpath not in options:
> -                                        options.append(newpath)
> -                                        default_option = len(options)
> -                                    else:
> -                                        default_option =
> options.index(newpath) + 1 +                                    options,
> default_option = add_to_options(options, newpath)
> 
>                              elif re.search('\d', ans):
>                                  default_option = ans
> @@ -2039,6 +2038,14 @@
>                              else:
>                                  done = False
> 
> +def add_to_options(options, newpath):
> +    if newpath not in options:
> +        options.append(newpath)
> +        default_option = len(options)
> +    else:
> +        default_option = options.index(newpath) + 1
A slower way would be only keep: default_option = options.index(newpath) + 1
and do away with the else and default_option = len(options) part.

Though its pretty inefficient for half the cases but would it be more readable 
(2 lines less to read ;-) )?

> +    return (options, default_option)
> +
>  def glob_path(newpath):
>      """Glob the given file path"""
>      if newpath[-1] == '/':
> 
> 

Thanks for the patch. Looks good in the given context.

Acked-by: Kshitij Gupta <kgupta8592 at gmail.com>.

Regards,

Kshitij Gupta

> 
> 
> Regards,
> 
> Christian Boltz
> 
> > [...] if the installation of a stupid package failed, [...]
> 
> AFAIK there is no package named `stupid'.
> [> Raphael Schillings and Michael Gross in
>  https://bugzilla.novell.com/show_bug.cgi?id=147588]