[apparmor] [patch 16/18] utils: address pep8 complaints

[Steve Beattie]

On Fri, Jan 17, 2014 at 01:29:31AM +0100, Christian Boltz wrote:
> Sorry for the terrible quoting, anyway:
> Does it really make sense to have two spaces in front of # ?

It's pep8's error 261. From running pep8 with the --show-pep8 argument:

  vim/create-apparmor.vim.py:91:96: E261 at least two spaces before inline comment
      Separate inline comments by at least two spaces.

      An inline comment is a comment on the same line as a statement.  Inline
      comments should be separated by at least two spaces from the statement.
      They should start with a # and a single space.

      Okay: x = x + 1  # Increment x
      Okay: x = x + 1    # Increment x
      E261: x = x + 1 # Increment x
      E262: x = x + 1  #Increment x
      E262: x = x + 1  #  Increment x

Now, I could be convinced to include E261 as another one to ignore, if
that is the group's consensus. I do want to add make targets that run
pyflakes and pep8 checks on our python code, so it'd be good to have
an agreed upon list; pyflakes in particular will catch some errors,
often times in exception cases that aren't always covered in testing.

> Am Donnerstag, 16. Januar 2014 schrieb Steve Beattie:
> > +#syn match  sdEntryM /@@DENYFILE@@(r|mk|x)+@@EOL@@/ 
> > contains=sdGlob,sdComment 
> > nextgroup=@sdEntry,sdComment,sdError,sdInclude
> > -#syn match  sdEntryM /@@DENYFILE@@(r|m|k|x)+@@EOL@@/
> > contains=sdGlob,sdComment 
> > nextgroup=@sdEntry,sdComment,sdError,sdInclude
> 
> NAK - you broke the regex here (first search for the difference 
> yourself, then have a look at [1] ;-)
> (and yes, I'd like to have this un-broken even if it's only a comment)

Bah, nice catch. I had thought about generating a diff without
whitespace changes to include with the submission. Doing so shows
it clearly (the result of running "qu diff --diff 'diff -uwB' before
addressing the issue):

=================
--- .pc/utils-pep8_cleanups.patch/utils/aa-easyprof	2014-01-16 16:34:58.569526130 -0800
+++ utils/aa-easyprof	2014-01-16 16:34:58.561526130 -0800
@@ -55,7 +55,7 @@
             files = [os.path.join(easyp.dirs['policygroups'], g)]
             apparmor.easyprof.print_files(files)
         sys.exit(0)
-    elif binary == None:
+    elif binary is None:
         error("Must specify full path to binary\n%s" % m)
 
     # if we made it here, generate a profile
--- .pc/utils-pep8_cleanups.patch/utils/vim/create-apparmor.vim.py	2014-01-16 16:34:58.569526130 -0800
+++ utils/vim/create-apparmor.vim.py	2014-01-16 16:34:58.561526130 -0800
@@ -36,10 +37,10 @@
     out, outerr = sp.communicate(input)
 
     # Handle redirection of stdout
-    if out == None:
+    if out is None:
         out = ''
     # Handle redirection of stderr
-    if outerr == None:
+    if outerr is None:
         outerr = ''
     return [sp.returncode,out+outerr]
 
@@ -146,7 +148,7 @@
 filerule = filerule + create_file_rule ( 'sdEntryM',   r'(r|m|k)+',  'mr - mmap with PROT_EXEC' )
 
 filerule = filerule + create_file_rule ( 'sdEntryM',   r'(r|m|k|x)+',  'special case: deny x is allowed (does not need to be ix, px, ux or cx)', 1)
-#syn match  sdEntryM /@@DENYFILE@@(r|m|k|x)+@@EOL@@/ contains=sdGlob,sdComment nextgroup=@sdEntry,sdComment,sdError,sdInclude
+#syn match  sdEntryM /@@DENYFILE@@(r|mk|x)+@@EOL@@/ contains=sdGlob,sdComment nextgroup=@sdEntry,sdComment,sdError,sdInclude
 
 
 filerule = filerule + create_file_rule ( 'sdError',    r'\S*(w\S*a|a\S*w)\S*',  'write + append is an error' )
=================

After fixing what you pointed out, the output of a non-whitespace-only diff looks like:

=================
--- .pc/utils-pep8_cleanups.patch/utils/aa-easyprof	2014-01-16 17:08:26.405573849 -0800
+++ utils/aa-easyprof	2014-01-16 17:08:26.397573849 -0800
@@ -55,7 +55,7 @@
             files = [os.path.join(easyp.dirs['policygroups'], g)]
             apparmor.easyprof.print_files(files)
         sys.exit(0)
-    elif binary == None:
+    elif binary is None:
         error("Must specify full path to binary\n%s" % m)
 
     # if we made it here, generate a profile
--- .pc/utils-pep8_cleanups.patch/utils/vim/create-apparmor.vim.py	2014-01-16 17:08:26.405573849 -0800
+++ utils/vim/create-apparmor.vim.py	2014-01-16 17:09:33.273575439 -0800
@@ -36,10 +37,10 @@
     out, outerr = sp.communicate(input)
 
     # Handle redirection of stdout
-    if out == None:
+    if out is None:
         out = ''
     # Handle redirection of stderr
-    if outerr == None:
+    if outerr is None:
         outerr = ''
     return [sp.returncode,out+outerr]
 
=================

> With that fixed, 
> Acked-by: Christian Boltz <apparmor at cboltz.de>

Thanks!

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20140116/2c6474cc/attachment.pgp>