[prev in list] [next in list] [prev in thread] [next in thread] 

List:       apparmor-dev
Subject:    Re: [apparmor] [PATCH] utils: Don't check for existence of abstraction files in aa-easyprof
From:       Tyler Hicks <tyhicks () canonical ! com>
Date:       2015-11-30 23:16:47
Message-ID: 20151130231646.GD10095 () boyd
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


On 2015-11-30 20:18:10, Christian Boltz wrote:
> Hello,
> 
> Am Sonntag, 29. November 2015 schrieb Tyler Hicks:
> > aa-easyprof is used to generate profiles and the lack of an
> > abstraction file during profile generation should not be an error
> > condition.
> > 
> > Leave the handling of the abstraction file for the parser. It will
> > fail if the file does not exist when the profile is being compiled.
> > 
> > https://launchpad.net/bugs/1521031
> 
> I understand why you want this change, and agree that raising an 
> exception in aa-easyprof is too heavy. OTOH, I don't like to silently 
> ignore missing abstractions.
> 
> What about printing a warning message? It won't hurt in build 
> environments (I'd guess nobody reads warnings - if something is serious, 
> it's an error ;-)  but if someone calls aa-easyprof manually, it can 
> still be helpful.

I'm fine with an error message.

I'll wait to hear back from responses to my other email to see if the
--include-abstractions-dir option is preferred over changing to a
warn().

Tyler

> 
> > --- a/utils/apparmor/easyprof.py
> > +++ b/utils/apparmor/easyprof.py
> > @@ -507,8 +507,6 @@ class AppArmorEasyProfile:
> >      def gen_abstraction_rule(self, abstraction):
> >          '''Generate an abstraction rule'''
> >          p = os.path.join(self.aa_topdir, "abstractions", abstraction)
> > -        if not os.path.exists(p):
> > -            raise AppArmorException("%s does not exist" % p)
> 
> better:
>     warn("%s does not exist" % p)
> 
> You'll need to import warn() from apparmor.common. Actually 
> apparmor.easyprof also has a warn() function - I seriously recommend to 
> de-duplicate it and import it from apparmor.common.
> 
> Speaking about de-duplicating - there are quite some functions in 
> apparmor.easyprof that are also in apparmor.common. Often with nearly 
> the same code, but it seems apparmor.commen received more bugfixes ;-)
> (comparing both files with meld is the easiest way to see the common 
> code)
> 
> 
> Regards,
> 
> Christian Boltz
> -- 
> Programming today is a race between software engineers striving to build
> bigger and better idiot-proof programs, and the Universe trying to
> produce bigger and better idiots. So far, the Universe is winning.
> 
> 
> -- 
> AppArmor mailing list
> AppArmor@lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor

["signature.asc" (application/pgp-signature)]

-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor


[prev in list] [next in list] [prev in thread] [next in thread]