[apparmor] [patch] remove unused LOG_MODE_RE in logparser.py

Christian Boltz apparmor at cboltz.de
Mon Jul 14 19:44:42 UTC 2014


Hello,

Am Montag, 14. Juli 2014 schrieb Seth Arnold:
> On Sat, Jul 12, 2014 at 09:29:14PM +0200, Christian Boltz wrote:
> > logparser.py defines LOG_MODE_RE, but doesn't use it.
> > 
> > LOG_MODE_RE is also defined (and used) in aamode.py.
> > 
> > This patch removes the superfluous definition from logparser.py.

> How confusing -- logparser.py has a LOG_MODE_RE variable it doesn't
> use but it does have a PROFILE_MODE_RE -- and aamode.py uses a
> LOG_MODE_RE variable? Should we be renaming variables along the way
> to make them make some kind of sense? 

Good question ;-)

I wouldn't be surprised if those definitions were copied from one file 
to another, which would explain why the names don't really match their
location.

Renaming them might be an option, but I'd say everybody who wants to 
rename something _first_ has to grep for duplicates and their usage, 
and, if possible, remove the duplicate.
If you do the rename first, finding the duplicate will be quite hard.

A quick grep indicates that some more regexes are _probably_ unused in 
logparser.py:
    MODE_MAP_RE = re.compile('r|w|l|m|k|a|x|i|u|p|c|n|I|U|P|C|N')
    PROFILE_MODE_RE = re.compile('r|w|l|m|k|a|ix|ux|px|cx|pix|cix|Ux|Px|PUx|Cx|Pix|Cix')
    PROFILE_MODE_NT_RE = re.compile('r|w|l|m|k|a|x|ix|ux|px|cx|pix|cix|Ux|Px|PUx|Cx|Pix|Cix')
    PROFILE_MODE_DENY_RE = re.compile('r|w|l|m|k|a|x')

Do you want to rename one of them? *eg*

Otherwise I'll double-check them and will send another patch when I'm 
sure they are really superfluous. (For now, I just commented them out 
in my checkout, so I'll notice if something breaks.)

Note: I only checked the lines around LOG_MODE_RE in logparser.py - 
feel free to hunt unused definitions at other places ;-)

> Or should these variables be
> defined in a single file that is then used by all the other files?

I prefer to have them defined near the place where they are used - that
makes understanding the code a bit easier ;-)

OTOH, having them at one place could make it easier to find (and merge)
nearly-duplicate regexes. Hmm...

> But this patch alone looks okay, so:
> 
> Acked-by: Seth Arnold <seth.arnold at canonical.com>

Thanks, commited.


Regards,

Christian Boltz
-- 
[18:16] <psankar> suseROCKs, sorry. didnt follow the conversation [...]
[18:16] <suseROCKs> psankar,   content is irrelevnt.  You are simply
here to nod yes to everyting I say with total and fervent agreement.
[from #opensuse-project]




More information about the AppArmor mailing list