[apparmor] deglobal-profiledir.diff (was Re: [patch] try to make subdomain.pm a little less smelly.)

[Jesse Michael]

On Wed, Oct 06, 2010 at 04:01:13PM -0700, Steve Beattie wrote:
> @@ -1747,6 +1747,9 @@ sub confirm_and_abort {
>      if ($ans eq "y") {
>          UI_Info(gettext("Abandoning all changes."));
>          shutdown_yast();
> +        foreach my $prof (@created) {
> +            delete_profile($prof);
> +        }
>          exit 0;
>      }
>  }
> 
> which I was about to propose for us to incorporate. However, unwinding
> the call chains to include $profiledir (and $sd) starts to get pretty
> complex, as it hits most of the UI layer. I can generate it the patch
> but it starts to return both variables to their global status if they
> just get passed around everywhere, perhaps pointing out a weakness of
> this approach. Is there any way we can encapsulate these in a way that
> they don't need to be passed all over the place?

Ahh, yeah, that's the sort of thing I was a little worried about.

Part of the reason I was hoping to get rid of the global variables
and make the variable passing explicit was to make it easier to 
figure out what was actually needed where and see what should be
reworked to make things cleaner.

It crossed my mind to pass around a struct containing the various
different important variables, but that's effectively the same thing as
having all those global variables if you need to pass the blob of all
of them around everywhere.  And doesn't give the benefits of making
the passing explicit.

One possible way to do it would be to handle the Abort case exactly
like the Finish case and just die with a special value which is used
to roll back any created profiles instead of calling save_profiles().
I'm not really thrilled with how the Finish case uses die to break out
of that many function levels to land in do_logprof_pass(), but it might
be better than passing $profiledir and $sd down to everything that might
call confirm_and_abort().

Ideally, I'd like to get rid of all of the eval uses except for directly
around when we're calling 3rd party library functions, the place where
eval is used to check regexp syntax, and maybe the Finish thing, if
there isn't a cleaner way to do that.  I think the multiple levels of
eval might be getting in the way of perl warning about mismatched 
prototypes in some cases.

And I think there's probably a bug there that the profile is still
left loaded into the kernel even though the file is deleted when Abort
is called.