Re: [PATCH 00/13] Fix some issues found by flake8

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Aug 6, 2018 at 5:05 PM, William Roberts
<bill.c.roberts@xxxxxxxxx> wrote:
>
> On Sat, Aug 4, 2018 at 12:47 PM, Nicolas Iooss <nicolas.iooss@xxxxxxx>
> wrote:
>>
>> Hi,
>>
>> I have been working on a script which uses flake8 to discover issues in
>> Python code. This led me to discover several issues which are fixed by
>> these patches. Distribution maintainers might be interested in
>> backporting some of them (at least patches 5 and 10 and probably the
>> ones which fix usage of undefined variables).
>>
>> As Travis-CI is experiencing issues today (it fails to launch new
>> builds), I have not been able to test the integration of my script with
>> Travis-CI yet. Once it works again I will submit this script too.
>>
>> Thanks,
>> Nicolas
>>
>> ---
>> Here comes the description generated by "git format-patch --cover":
>>
>> Nicolas Iooss (13):
>>   libselinux: fix flake8 warnings in SWIG-generated code
>>   python/sepolgen: do not import twice the modules
>>   python/sepolgen: return NotImplemented instead of raising it
>>   python/sepolicy: drop unused CheckPolicyType
>>   python/sepolicy: use lowercase variable name
>>   python/sepolgen: fix refpolicy parsing of "permissive"
>>
>>   python/sepolgen: silence linter warning about has_key
>
>
> This is the only one I don't particularly like:
>>
>>   python/sepolgen: comment buggy code
>
>
> If we choose to comment out it out, a block comment explaining why would be
> helpful or just delete it if it is dead code.

It feels like this code could be useful if an interface parameter
("$1") is used as a permission, which is why I have not deleted it. I
suggest adding the following comment:

    # This function currently ignores parameters which are used in permission.
    # The following code has been present for a long time and contains two
    # syntax errors (__param_insert takes 4 arguments and PERM is not defined)
    # which proves that it is actually dead code.
    #for perm in av.perms:
    #    if access.is_idparam(perm):
    #        if __param_insert(perm, PERM) == 1:
    #            ret = 1

Does this suit you? By the way, I do not have a strong opinion about
commenting or deleting it, so I am fine if we choose to remove this
code instead.

Thanks for your review,
Nicolas

_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.



[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux