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 <st...@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]
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
