[apparmor] [patch] split off serialize_parse_profile_start_line()

[Christian Boltz]

Hello,

Am Montag, 9. März 2015 schrieb Steve Beattie:
> On Tue, Mar 03, 2015 at 11:43:28PM +0100, Christian Boltz wrote:
> > this patch splits off serialize_parse_profile_start_line() from 
> > serialize_profile_from_old_profile() in aa.py, as a preparation to
> > add  tests and then switch to the upcoming RE_PROFILE_START wrapper
> > function.

> I'm okay with this as is, so Acked-by: Steve Beattie <steve at nxnw.org>
> for trunk and 2.9.
> 
> However, a couple of things make me scratch my head a bit:
> 
>   1) I can't tell entirely from the complicated regex and from the
> code paths, but is it possible that the boolean value of
> 'in_contained_hat' can be derived from the other return values?

Good question, I'd have to dig deeper in the code to answer that.

>   2) I don't get the purpose of the correct variable and wonder if
>      we should be raising an exception here?

Maybe the variable "just" has a strange name, but again, I'd have to dig 
much deeper to answer that in a correct ;-) way. A quick comparison with 
parse_profile_start() indicates that "correct == False" could mean an 
external hat, but without the prof_data_external flag set.

The tests added in the next patch probably explain the behaviour best - 
"correct == False" happens
- for external hats ("/foo//bar {") - which actually _is_ correct syntax
- for external hats inside a profile (this one is probably worth an 
  exception, see the comment in the tests)

This means we should probably rename "correct" to "external_hat" and add 
an exception if we find an external hat definition inside a profile 
(that will probably never be hit because invalid syntax like that should 
already be catched when reading the profiles ;-)


That said - the purpose of this patch was to split out a part from 
serialize_profile_from_old_profile() so that I could write tests in a 
sane way, and to keep the risk if breaking 
serialize_profile_from_old_profile() (which is not covered by tests yet) 
as low as possible.

If we have full test coverage for serialize_profile_from_old_profile() 
one day, I'll happily hunt down all those gremlins - but without tests, 
I'm not too keen to do big changes in such a big function ;-)

For now, I'll stop thinking about serialize_parse_profile_start() (and 
it's nearly-copy parse_profile_start()) because that uncovers too many 
bugs ;-)


Regards,

Christian Boltz
-- 
Guten Tag. Ich will ein Haus bauen. Was soll ich verwenden:
Steine oder Mörtel?      [Kristian Koehntopp in suse-linux]