[Freeipa-devel] [PATCHES] 0696-0710 More modernization

Wed Aug 26 10:15:44 UTC 2015

On 08/26/2015 09:47 AM, Jan Cholasta wrote:
> On 25.8.2015 15:05, Petr Viktorin wrote:
>> On 08/25/2015 12:39 PM, Christian Heimes wrote:
>>> On 2015-08-24 17:31, Petr Viktorin wrote:
>>>>>>> 0701.2-Use-Python3-compatible-dict-method-names
>>>>>>> NACK
>>>>>>> Why are you replacing iteritems() with items() instead of using
>>>>>>> six.iteritems()?
>>>>
>>>> It looks cleaner, and it will be easier to clean up after six is
>>>> dropped.
>>>> Also, the performance difference is negligible if the whole thing is
>>>> iterated over. (On small dicts, which are the majority of what
>>>> iteritems
>>>> was used on, items() is actually a bit faster on my machine.)
>>>
>>> Right, for small dicts the speed difference is negligible and favors the
>>> items() over iteritems(). For medium sized and large dicts the iterators
>>> are faster and consume less memory.
>>>
>>> I'm preferring iterator methods everywhere because I don't have to worry
>>> about dict sizes.
>>>
>>>>>>> 0710.2-Modernize-use-of-range
>>>>>>> NACK
>>>>>>> Please use six.moves.range. It defaults to xrange() in Python 2.
>>>>
>>>> Why? What is the benefit of xrange in these situations?
>>>>
>>>> Like with iteritems in 0701, when iterating over the whole thing, the
>>>> performance difference is negligible. I don't think a few microseconds
>>>> outside of tight loops are worth the verbosity.
>>>
>>> It's the same reasoning as in 0701. As long as you have a small range,
>>> it doesn't make a difference. For large ranges the additional memory
>>> usage can be problematic.
>>>
>>> In all above cases the iterator methods and generator functions are a
>>> safer choice. A malicious user can abuse the non-iterative methods for
>>> DoS attacks. As long as the input can't be controlled by a user and the
>>> range/dict/set/list is small, the non-iterative methods are fine. You
>>> have to verify every location, though.
>>
>> Keep in mind that for dicts, all the data is in memory already (in the
>> dict). Are you worried about DoS from an extra list of references to
>> existing objects?
>>
>>> I'm usually too busy with other stuff (aka lazy) to verify these
>>> preconditions...
>>
>> I changed one questionable use of range. The other ones are:
>>
>> ipalib/plugins/dns.py: for i in range(len(relative_record_name)
>> (max. ~255, verified by DNSName)
>>
>> ipaserver/install/dnskeysyncinstance.py: for _ in range(0, 16))
>> (16)
>>
>> ipaserver/install/ipa_otptoken_import.py: for i in range(1, blocks + 1):
>> (about 7)
>>
>> ipaserver/install/ipa_otptoken_import.py: for k in
>> range(mac.digest_size):
>> (16)
>>
>> ipatests/data.py: for d in range(256))
>> (256)
>>
>> Plus tests, where it's rarely above 10.
>>
> 
> I have just pushed Michael's python-krbV removal patch, which conflicts
> with your patches. Could you please rebase them on top of current master?
> 

Sure, here they are.

-- 
Petr Viktorin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0696.5-Remove-use-of-sys.exc_value.patch
Type: text/x-patch
Size: 2529 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150826/5ed1c406/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0697.5-Don-t-use-a-tuple-in-function-arguments.patch
Type: text/x-patch
Size: 907 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150826/5ed1c406/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0698.5-Add-python-six-to-dependencies.patch
Type: text/x-patch
Size: 984 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150826/5ed1c406/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0699.5-Remove-the-unused-pygettext-script.patch
Type: text/x-patch
Size: 30062 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150826/5ed1c406/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0700.5-Use-six.string_types-instead-of-basestring.patch
Type: text/x-patch
Size: 35449 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150826/5ed1c406/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0701.5-Use-Python3-compatible-dict-method-names.patch
Type: text/x-patch
Size: 73485 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150826/5ed1c406/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0702.5-Replace-filter-calls-with-list-comprehensions.patch
Type: text/x-patch
Size: 6514 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150826/5ed1c406/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0703.5-Use-six.moves.input-instead-of-raw_input.patch
Type: text/x-patch
Size: 3100 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150826/5ed1c406/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0704.5-Use-six.integer_types-instead-of-long-int.patch
Type: text/x-patch
Size: 8301 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150826/5ed1c406/attachment-0008.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0705.5-Replace-uses-of-map.patch
Type: text/x-patch
Size: 18773 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150826/5ed1c406/attachment-0009.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0706.5-Use-next-function-on-iterators.patch
Type: text/x-patch
Size: 3481 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150826/5ed1c406/attachment-0010.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0707.5-Use-the-print-function.patch
Type: text/x-patch
Size: 229158 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150826/5ed1c406/attachment-0011.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0708.5-Use-new-style-raise-syntax.patch
Type: text/x-patch
Size: 6956 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150826/5ed1c406/attachment-0012.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0709.5-Use-six.reraise.patch
Type: text/x-patch
Size: 1892 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150826/5ed1c406/attachment-0013.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0710.5-Modernize-use-of-range.patch
Type: text/x-patch
Size: 18088 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150826/5ed1c406/attachment-0014.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0711.5-Convert-zip-result-to-list.patch
Type: text/x-patch
Size: 1223 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150826/5ed1c406/attachment-0015.bin>