[apparmor] [patch] new profile tools - handling of "(F)inish"

[Kshitij Gupta]

On Feb 25, 2014 12:15 AM, "Christian Boltz" <apparmor at cboltz.de> wrote:
>
> Hello,
>
> [patch v2, see below]
>
> Am Montag, 27. Januar 2014 schrieb Christian Boltz:
> > currently, selecting (F)inish in the new profile tools basically means
> > aborting without saving anything. However, we already have Abo(r)t
> > for that ;-)
> >
> > (F)inish should ask the user if he wants to save the changed profiles
> > before exiting.
> >
> > The attached patch does this (except for two places, where I just
> > added a TODO instead of real code ;-) - probably solvable by
> > copy&paste)
>
> Ignore the "except ..." part - that's fixed in the v2 patch ;-)
>
> > I don't really like that I had to remove the central handling of
> > (F)inish in ui.py and add it to several places in aa.py. OTOH calling
> > confirm_and_finish() from ui.py a) isn't that easy and b) would mix
> > program logic into ui.py - I'm not sure if this is a good idea.
> >
> > Hmm, maybe we should also move the handling for Abo(r)t to aa.py and
> > make ui.py totally dump to be consistent, even if this means to add
> > some duplicated lines?
> >
> > Or add a small function to aa.py as wrapper for UI_PromptUser that
> > - always adds (F)inish and Abo(r)t
> > - handles those two
> > - and returns any other answer to the calling function
> > ?
> > This function could then be used for all user prompts, with the
> > exception of the "save profile / view diff" prompt.
> >
> > Opinions?
>
Sounds good. And as for save profile prompt, it needs some work as
discussed.
>
> Here's an updated version of the patch that applies to current bzr trunk.
>
> === modified file 'utils/apparmor/aa.py'
> --- utils/apparmor/aa.py        2014-02-24 18:20:11 +0000
> +++ utils/apparmor/aa.py        2014-02-24 18:42:03 +0000
> @@ -498,6 +498,10 @@
>
>      ans = ''
>      while 'CMD_USE_PROFILE' not in ans and 'CMD_CREATE_PROFILE' not in
ans:
> +        if ans == 'CMD_FINISHED':
> +            save_profiles()
> +            return;
> +
>          ans, arg = aaui.UI_PromptUser(q)
>          p = profile_hash[options[arg]]
>          q['selected'] = options.index(options[arg])
> @@ -990,6 +994,10 @@
>
>                      ans = aaui.UI_PromptUser(q)
>
> +                    if ans == 'CMD_FINISHED':
> +                        save_profiles()
> +                        return;
> +
>                  transitions[context] = ans
>
>                  if ans == 'CMD_ADDHAT':
> @@ -1251,6 +1259,11 @@
>                                  q['functions'] +=
build_x_functions(default, options, exec_toggle)
>                                  ans = ''
>                                  continue
> +
> +                            if ans == 'CMD_FINISHED':
> +                                save_profiles()
> +                                return;
> +
>                              if ans == 'CMD_nx' or ans == 'CMD_nix':
>                                  arg = exec_target
>                                  ynans = 'n'
> @@ -1550,6 +1563,11 @@
>                      done = False
>                      while not done:
>                          ans, selected = aaui.UI_PromptUser(q)
> +
> +                        if ans == 'CMD_FINISHED':
> +                            save_profiles()
> +                            return;
> +
>                          # Ignore the log entry
>                          if ans == 'CMD_IGNORE_ENTRY':
>                              done = True
> @@ -1794,6 +1812,10 @@
>
>                              ans, selected = aaui.UI_PromptUser(q)
>
> +                            if ans == 'CMD_FINISHED':
> +                                save_profiles()
> +                                return;
> +
>                              if ans == 'CMD_IGNORE_ENTRY':
>                                  done = True
>                                  break
> @@ -1945,6 +1967,11 @@
>                          done = False
>                          while not done:
>                              ans, selected = aaui.UI_PromptUser(q)
> +
> +                            if ans == 'CMD_FINISHED':
> +                                save_profiles()
> +                                return;

I don't like the ";" there after return. Besides that it looks good. Not
tested but I believe you did.

Acked-by: Kshitij Gupta, maybe remove the semicolon (Matter of taste).

> +
>                              if ans == 'CMD_IGNORE_ENTRY':
>                                  done = True
>                                  break
>
> === modified file 'utils/apparmor/ui.py'
> --- utils/apparmor/ui.py        2014-02-13 18:01:03 +0000
> +++ utils/apparmor/ui.py        2014-02-24 18:32:40 +0000
> @@ -288,10 +288,6 @@
>      if cmd == 'CMD_ABORT':
>          confirm_and_abort()
>          cmd = 'XXXINVALIDXXX'
> -    elif cmd == 'CMD_FINISHED':
> -        if not params:
> -            confirm_and_finish()
> -        cmd = 'XXXINVALIDXXX'
>      return (cmd, arg)
>
>  def confirm_and_abort():
> @@ -317,9 +313,6 @@
>                      })
>      ypath, yarg = GetDataFromYast()
>
> -def confirm_and_finish():
> -    sys.stdout.write(_('FINISHING...\n'))
> -    sys.exit(0)
>
>  def Text_PromptUser(question):
>      title = question['title']
>
>
>
> Regards,
>
> Christian Boltz
> --
> Wenn ich mir ein System installiere, welches dieses oder jenes oder
> welches "für mich" tut (gegen mich?), dann kann ich gleich Windows
> nehmen. Das fährt sich so gut, wie ein elektronischer Chauffeur das
> eben kann: Nämlich bis die Elektronik ein Dixi-Klo mit 'ner Garage
> verwechselt: Eckig und die Tür war offen.   [Ratti in suse-linux]
>
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20140225/442a3c32/attachment-0001.html>