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:
Why are you replacing iteritems() with items() instead of using

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.

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

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))

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):

ipatests/data.py: for d in range(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?

Jan Cholasta

Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to