[apparmor] [PATCH v2 3/8] utils: Require apparmor.aa users to call init_aa()

[Tyler Hicks]

On 03/02/2017 01:32 PM, Christian Boltz wrote:
> Hello,
> 
> Am Mittwoch, 1. März 2017, 21:52:01 CET schrieb Tyler Hicks:
>> Introduce an apparmor.aa.init_aa() method and move the initialization
>> code of the apparmor.aa module into it. Note that this change will
>> break any external users of apparmor.aa because global variables that
>> were previously initialized when importing apparmor.aa will not be
>> initialized unless a call to the new apparmor.aa.init_aa() method is
>> made.
> 
> 
>> diff --git a/utils/aa-mergeprof b/utils/aa-mergeprof
>> index 4e1e633..1241515 100755
>> --- a/utils/aa-mergeprof
>> +++ b/utils/aa-mergeprof
>> @@ -43,6 +43,8 @@ args = parser.parse_args()
>>
>>  args.other = None
>>
>> +apparmor.aa.init_aa()
>> +
>>  profiles = args.files
>>
>>  profiledir = args.dir
>> @@ -136,6 +138,7 @@ class Merge(object):
>>          user, base = profiles
>>
>>          #Read and parse base profile and save profile data, include
>> data from it and reset them 
>> +        apparmor.aa.init_aa()
>>          apparmor.aa.read_profile(base, True)
>>          self.base = cleanprofile.Prof(base)
> 
> Just curious - what's the reason for calling init_aa() a second time?
> I didn't test, but I'd guess that doing it in the global code should be 
> enough.

Good catch. I meant to circle back to aa-mergeprof and make a final
decision on whether it was best to do it in the global code or in the
class constructor. I agree that doing it in the global code is
sufficient. I'm going to drop the call from the constructor.

> 
> 
>> --- a/utils/test/Makefile
>> +++ b/utils/test/Makefile
> 
>>  check: __libapparmor
>> -	export PYTHONPATH=$(PYTHONPATH) ; export
>> LD_LIBRARY_PATH=$(LD_LIBRARY_PATH) ; export LC_ALL=C; $(foreach test,
>> $(wildcard test-*.py), echo ; echo === $(test) === ; $(call pyalldo,
>> $(test))) 
>> +	export PYTHONPATH=$(PYTHONPATH)
>> LD_LIBRARY_PATH=$(LD_LIBRARY_PATH) LC_ALL=C __AA_CONFDIR=$(CONFDIR) ;
>> $(foreach test, $(wildcard test-*.py), echo ; echo === $(test) === ;
>> $(call pyalldo, $(test)))
> 
> I remember discussions about line lenghts in python. Did we already have 
> such a discussion about Makefiles? ;-)

Hmm? I don't recall what you're referring to. Are you wanting me to wrap
the lines that I modified?

I strongly prefer to follow existing conventions when making feature
changes or bug fixes and leave coding style cleanups to separate patches.

> (I know changing this in this patch would break the following patches, 
> so if you want shorter lines, feel free to send a follow-up patch.)

FYI, this sort of thing isn't a problem with git rebase (I use
git-remote-bzr for apparmor devel).

> 
> Both questions shouldn't stop you from commiting, so
> Acked-by: Christian Boltz <apparmor at cboltz.de>

Thanks!

Tyler

> 
> 
> Regards,
> 
> Christian Boltz
> 
> 
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20170302/4b1b58c0/attachment.pgp>