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

[Kshitij Gupta]

Hello,

On Monday, August 25, 2014 10:36:54 PM Christian Boltz wrote:
> Hello,
> 
> Am Dienstag, 26. August 2014 schrieb Kshitij Gupta:
> > On Sunday, August 24, 2014 04:44:40 PM Christian Boltz wrote:
> > > 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.
> > > 
> > > +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 ;-) )?
> 
> Sounds like a good idea, and the performance doesn't really matter in
> this case (we are talking about maybe 20 lines that are executed before
> waiting for the next user input)
> 
> 
> Updated patch:
> 
> === modified file 'utils/apparmor/aa.py'
> --- utils/apparmor/aa.py        2014-08-25 20:05:45 +0000
> +++ utils/apparmor/aa.py        2014-08-25 20:33:29 +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,13 @@
>                              else:
>                                  done = False
> 
> +def add_to_options(options, newpath):
> +    if newpath not in options:
> +        options.append(newpath)
> +
> +    default_option = options.index(newpath) + 1
> +    return (options, default_option)
> +
>  def glob_path(newpath):
>      """Glob the given file path"""
>      if newpath[-1] == '/':
> 
> 
> 

Thanks for the updated patch. :-)

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

Regards,

Kshitij Gupta

> 
> Regards,
> 
> Christian Boltz