On Sat, May 30, 2015 at 12:21:40AM +0200, Christian Boltz wrote: > Hello, > > Am Freitag, 29. Mai 2015 schrieb Steve Beattie: > > On Sun, May 24, 2015 at 05:59:58PM +0200, Christian Boltz wrote: > > > this patch adds severity() to the BaseRule class > > > > > > severity() will, surprise!, return the severity of a rule, or > > > '--' if a *Rule class doesn't implement the severity() function > > > > This seems like an error state/bug if it's not implemented. Wouldn't > > it be better to raise AppArmorBug() here? We already do this for > > other unimplemented abstract methods (which is also a hint that the > > method should get the FIXME comment like the other abstract methods > > have). > > I can imagine why you are asking, but I tend to say no ;-) > > Even if it would be nice to have it for all rule types, severity() is an > optional feature (as opposed to _parse() or _is_covered_localvars()), > so if we change it to raise an exception, we'll need something like > *Rule.features = ['severity'] to give the calling code a way to prevent > calling a non-implemented severity(). That would make the code more > complex (we need severity() _and_ features[], with the risk that > features[] gets out of sync) without a real win.
Or uh you could just catch the exception and treat it as an unknown severity. (It would have the benefit of continuing on in the face of other severity.db failures as well.) The other way to approach it would be to have .severity() return the actual value that severity db treats as unknown, rather than ginning up yet another magic value. But I'd rather push that into the Rules subclasses themselves, so that at least the author of the subclass thinks about whether or not to implement something more sophisticated for the severity db than just returning essentially unknown. -- Steve Beattie <[email protected]> http://NxNW.org/~steve/
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
