[ Sending again as my mail Sat, Sep 05, 2009 at 11:00:46AM +0200 didn't seem to have reach the list, Daniel ] On Fri, Sep 04, 2009 at 04:03:50PM -0500, Jamie Strandboge wrote:

On Fri, 04 Sep 2009, Daniel Veillard wrote:

...

On Fri, Sep 04, 2009 at 12:23:33PM -0500, Jamie Strandboge wrote:

...

This patch series implements the AppArmor security driver for sVirt. This implementation was developed for the Ubuntu AppArmorLibvirtProfile specification[1], but is general enough for any AppArmor deployment (such as Ubuntu, *SUSE and Mandriva).

This patch has seen quite a bit of real world testing in Ubuntu 9.10 (our development release) in our 0.7.0-1ubuntu3 package. I did make a few small changes after going through HACKING, but mostly I got the tests going and added documentation.

Could you triple check that the make syntax-check run clean with your patches applied, because a very quick glimpse shows up a number of malloc() which we don't allow for example. It may be because you didn't enter the files in the SCM but the checks should be done anyway on the new files please.

I noticed that syntax-check wouldn't scan the new files unless I did a git commit first. I did not run 'syntax-check' after adding the documention examples, and just found that examples/apparmor/libvirt-qemu had a trailing blank line (I was a bit surprised syntax-check checked these files (attached is an updated docs patch)).

okay

I can demonstrate that the files are being checked by removing the '#include <config.h>' line in security_apparmor.c and virt-aa-helper.c and syntax-check failing: [...] I don't see that 'syntax-check' enforces not using *alloc, but I did read in HACKING that *alloc are deprecated, though I had already written the code before reading it. I do see other examples of *alloc in the

I really though all cases of direct malloc or realloc had been eliminated, apparently it's not the case yet, and syntax-check doesn't enforce it... we need to do this. I note we also still have a few free() left around in the code base to cleanup too.

codebase, and I do properly check the return code of all calls to *alloc. If you insist on those calls being replaced, I can do that, but I didn't before submitting because I knew the current calls were correct and didn't want to introduce bugs into the code.

yeah, I think it's better to fix them before. In any case we have a week of bug fix only before the next release in a week, so best is to fix them now, wait for the actual reviews and then resubmit patches for inclusion. thanks, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/